OLMIS-4798: Updated Git, Branching & Pull Requests section in the contribution...

Activity

FEOLMIS-3029 12

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 66h 41m 4 Sebastian Brudziński 1.Yes, we have: http://docs.openlmis...
    Reviewer - Complete 4m    
    Reviewer - 100% reviewed 22m 8 The part on pull requests looks good to me, but I also ad...
    Reviewer - Complete 4m    
    Reviewer - 100% reviewed 5m    
    Total   67h 16m 12  
    #permalink

    Objectives

    There are no specific objectives for this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Sebastian Brudziński

    Klaudia Pałkowska I think there's some confusion on how the pull request work...

    Klaudia Pałkowska I think there's some confusion on how the pull request workflow should look like and work in OpenLMIS, especially concerning when code reviews happen. My understanding was that we are doing code reviews on the pull requests themselves when they are submitted. Once they are merged, they wouldn't have an additional code review (why would it, if it has already been reviewed?). Also PRs would mainly be aimed at the new/external developers, so they have a way to contribute to the codebase and have their code reviewed before merging into master.

    Klaudia Pałkowska

    That is what I meant. The confusion is probably caused by using code review t...

    That is what I meant. The confusion is probably caused by using code review term in case of pull requests. I'll try to change it somehow to be more clear.

    Klaudia Pałkowska

    Sebastian Brudziński I updated the review with last changes.

    Sebastian Brudziński I updated the review with last changes.

    Sebastian Brudziński

    Thanks, this looks better.

    Thanks, this looks better.

    Sebastian Brudziński

    Klaudia Pałkowska do we already have info on release branches? If not, it wou...

    Klaudia Pałkowska do we already have info on release branches? If not, it would be good to add it. Also, did you take a look at the https://trunkbaseddevelopment.com? Is there anything worth adding there?

    Klaudia Pałkowska

    Sebastian Brudziński 1.Yes, we have: http://docs.openlmis.org/en/latest/conve...

    Sebastian Brudziński
    1.Yes, we have: http://docs.openlmis.org/en/latest/conventions/versioningReleasing.html#patch-releasing-a-component.
    2. Yes, the documentation was written based on the https://trunkbaseddevelopment.com/. In the last commit I've added information about number of developers on the same short-lived branch.

    Sebastian Brudziński

    The part on pull requests looks good to me, but I also added Josh Zamor as an...

    The part on pull requests looks good to me, but I also added Josh Zamor as an optional reviewer. It's also worth noting that the docs contain info on features we don't have yet, but are working on them (eg. builds & tests for PRs).

    /docs/.../contribute/contributionGuide.md Changed 5

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against