Skip to content

Team Norms

asharonbaltazar edited this page Dec 9, 2024 · 1 revision

Pull Requests

Introduction

Pull Requests (PRs) are an essential part of our software development process, enabling code review, discussion, and integration. This document outlines our team norms regarding PRs to ensure a smooth, efficient, and collaborative workflow.

Above all else, please remember: your discretion and judgement supersedes this document. If you’re creating or reviewing a Pull Request that needs to break any of these norms for good reason, that’s OK. This document is the rule; exceptions are expected.

Creating Pull Requests

Content and Format

  • Title: The title of your PR should be carefully considered. It should be descriptive enough to convey the essence of the changes but concise enough to be easily readable. Think of the title as a brief summary that helps others quickly understand the purpose of the PR.
  • Follow the Template: We use a Pull Request template to standardize and streamline our PR descriptions. It is expected that all PRs will adhere to this template.

Small, Focused Changes

  • Scope: Keep PRs focused on a single task or issue to simplify review and discussion. While most pull requests will each address a single ticket, it’s OK to handle multiple Jira tickets in one PR when it makes sense.
  • Size: Aim for small, manageable PRs. Large PRs should be broken down into smaller parts if possible.
  • Formatters: Be conscious of formatting updates that your IDE may make automatically, or that you may make along the way. Sometimes small, non-functional code changes can clutter a pull request.

Reviewing Pull Requests

Timelines

  • Prompt Reviews: Team members are expected to review PRs in a timely manner, typically next day or sooner.

Constructive Feedback

  • Respectful Communication: Provide constructive, respectful feedback.
  • Specific Comments: Use specific comments to point out issues, suggest improvements, or ask clarifying questions.

Testing

Test as Appropriate: The burden of full end to end testing lies with the author, our automated testing frameworks, and any manual QA process. However, reviewers should test the environment when they think it necessary, perhaps when they’ve thought of an edge case that might not be covered. Explain your testing: The PR approach should typically include some detail around how manual tests were performed. This helps greatly in allowing reviewers to sign off without needing to test individually.

Merging Pull Requests

Passing Checks

  • CI/CD: Ensure all continuous integration and deployment checks have passed.
  • Code Quality: Code should meet our standards for quality and maintainability.

Approval

  • Minimum Approvals: PRs require a minimum number of approvals (1) from the team members before merging.
  • Outstanding Comments: If there are comments that ask for action or consideration to be made by the author, please address them prior to merge, regardless if you have approval.

Merging

  • Author Merges: After receiving necessary approvals, the PR author is responsible for merging the code.
  • Squashing Protocol: When merging into the master branch, always squash and merge. When merging into val and production, create a merge commit.
  • Commit Messages: We use semantic release to automatically release our product. Semantic Release looks for commit messages with special commit message syntax. Please follow this syntax when crafting your commit message. Note: GitHub will use your PR title as the default commit message when squashing; so, it’s recommended to set your PR title equal to the semantic release commit message appropriate for your changeset.

Responsibility and Accountability

  • Ownership: The author is responsible for addressing feedback and ensuring the PR is ready for merging.
  • Collaboration: All team members share the responsibility for maintaining a high standard of code through thoughtful PR reviews.

Code Style

What is it?

A set of rules or guidelines used when writing the source code for a computer program.

Why is it?

It keeps the code looking neat and tidy, so anyone on the team can jump in and not get lost in a jungle of curly braces and indentation levels. It’s like everyone speaking the same slang in a super cool secret club. Plus, it saves time from arguing over trivial stuff, like whether tabs are better than spaces, so there’s more time for the fun stuff—coding and creating!

OneMAC Style Norms

Cheatsheet

TL;DR? No worries, here’s a cheatsheet of the concepts outlined below:

DO NOT destructure so we maintain object context for methods and properties used in code

Object Access

When integrating with complex objects, consider maintaining the object’s integrity rather than opting for destructuring. This approach ensures that the object’s context is preserved, enhancing readability and maintainability. Nomenclature simplification

For instance, rather than breaking down the object into individual variables, which can lead to verbose and confusing naming conventions, maintain the object as a whole.

const { setModalOpen, setContent: setModalContent, setOnAccept: setModalOnAccept, } = useModalContext(); 
const { setContent: setBannerContent, setBannerShow, setBannerDisplayOn, } = useAlertContext();

// vs

const modal = useModalContext(); 
const alert = useAlertContext();

Usage implication

This method simplifies reference to its properties and methods, providing a clearer and more direct understanding of its usage within the code. This strategy is particularly beneficial in scenarios where the object's structure and context significantly contribute to its functionality and meaning in the application.

<form
  onSubmit={form.handleSubmit(async (data) => {
    try {
      await submit({
        //...
      });
      alert.setContent({
        header: "RAI response submitted",
        body: `The RAI response for ${item._source.id} has been submitted.`,
      });
      alert.setBannerShow(true);
      alert.setBannerDisplayOn("/dashboard");
      navigate({ path: "/dashboard" });
    } catch (e) {
      //...
    }
  })}
>

JIRA Workflow

Introduction

This document outlines our team’s process for managing tasks using the JIRA Kanban board. It’s designed to provide clarity and consistency on how we track and progress through work items.

Kanban Columns

Backlog

  • Population: The backlog is populated with new tasks during the planning phase.
  • Prioritization: Tasks are prioritized based on urgency and importance.

Ready

  • Preparation: Tasks are moved to “Ready” once they are clearly defined and ready for development.
  • Final Checks: Ensure that all necessary information and acceptance criteria are present. Story points should be added prior to moving to Ready.

In Progress

  • Active Development: When a task is actively being worked on, it is moved to “In Progress”.
  • Daily Updates: Developers should provide daily updates or comments on the task to indicate progress.

In Review

  • Code Review: Tasks are moved here when the development is complete and they are awaiting code review.
  • Peer Feedback: Team members provide feedback and approval. If feedback requires a significant modification or rewrite to the code, the ticket should be moved back to In Progress.

In Testing

  • Quality Assurance: Tasks in this column are undergoing thorough testing by the QA team.
  • Env URL: When a developer moves a ticket into In Testing, the dev should make a comment on the ticket that includes the deployed environment’s URL, as well as probably tagging the QA team for convenience.
  • Hands Off: Developers should not push code that updates environments for work that is In Testing, without coordinating with the QA team. This is to prevent deployments interfering with the QA process.
  • Bug Reporting: Any issues discovered during testing are reported and linked to the task.
  • Failures: If a ticket fails QA for reasons that should not be addressed separately (like a bug), the QA team will move the ticket back to In Review.

Ready for Merge

  • Final Approval: Once testing is complete and the task has passed all checks, it moves to “Ready for Merge”.
  • Merge Preparation: The task is prepared for merging into the main codebase.

In Pipeline

  • Deployment: Tasks here have been merged to master, and are in the process of being verified on master by the QA team.
  • Monitoring: Close monitoring of the feature in the live environment for any immediate issues.

Done

  • Completion: Tasks are moved to “Done” when they are merged to master and verified on master, if applicable.
  • Review: The team may review completed tasks to ensure all objectives were met.
  • Demo Coordination: If the completed work is going to be demoed, coordinate a time with Product to relay the context of the feature and how to demonstrate it.

Sprint Tracking

Current and Upcoming Sprints

  • Active Sprint: Tasks currently being worked on are tracked under the active sprint, e.g., “Sprint 2.4”.
  • Future Sprints: Upcoming work items are assigned to future sprints, and periodically rescheduled during refinement as events unfold.

Sprint Review

  • Regular Reviews: At the end of each sprint, the team reviews progress and adjusts future plans accordingly.
  • Continuous Improvement: Lessons learned are discussed and processes are adjusted to improve future sprints.

Responsibility and Accountability

  • Ownership: Team members take ownership of tasks they are working on and update their status accordingly.
  • Collaboration: The entire team collaborates to ensure tasks move smoothly through the workflow.

By adhering to this JIRA workflow, we aim to maintain a high level of organization and efficiency within our development process.