search

NOAA-FIMS / collaborative_workflow

contributors guide to FIMS, managing collaborations
https://noaa-fims.github.io/collaborative_workflow/
4 stars 1 forks source link

clarify goals of code review #119

Closed iantaylor-NOAA closed 2 weeks ago

iantaylor-NOAA commented 5 months ago

The code review process is briefly described at https://noaa-fims.github.io/collaborative_workflow/fims-project-management-process.html#reviewers. However, at the 2023 in-person workshop, the documentation discussion identified this action item "How to get communication on what the goal of the code review is.".

I no longer remember the discussion around that note, but I think more information on the goals of code review would be useful in that Collaborative Workflow section, which hasn't been edited since May 2022.

ChristineStawitz-NOAA commented 5 months ago

The code review process for MQ led to a lot of PRs open for a long time as code reviews take a while, so clarifying here should include lowering expectations around style and docs for an active development milestone

Bai-Li-NOAA commented 3 months ago

We will discuss a potential flowchart for code review, based on discussions from the 2023 in-person workshop, during the M2 documentation team meeting on 03/11/2024 (running notes).

graph TD;
    A["Author: Submit a pull request (PR)\n\n#bull; Documentation included\n#bull; Tests implemented\n#bull; CI checks passed"] --> B["Author: Propose a reviewer\n\n#bull; Default assignee: an OST rep for a quick review"];
    B --> C["Reviewer: Receive code review request"];
    C --> |No assistance needed| E["Reviewer: Execute code review"];
    C --> |Help wanted| D["Reviewer: Pair an OST rep with a center rep"] --> E;
    E --> |Changes needed| F["Reviewer: Request changes"] --> G["Author: Make changes"] --> H["Reviewer: Approve PR"];
    E --> |No changes needed| H;
    H --> I[Author: Merge PR];

style A text-align:left