Skip to content

Pull Request Workflow

Jump to bottom
madbkr edited this page Oct 3, 2024 · 3 revisions

Overview

This page describes ...

  • how to create a PR (developer-guide)
  • how to review a PR (reviewer-guide)

Important Remarks

⚠: In case you're not familiar with the pull request (PR) functionality on GitHub, read through the general PR-Guide. It's also a perfect refresher for your existing knowledge.

The OEO-PR-workflow differs from the standard PR-workflow you might know from other projects!

Be aware that these are the general rules, but exception might apply. In doubt, ask for help from the @oeo-release-team what to do. They're most familar with the process.

Workflow

Roles

A pull request requires action from multiple people:

  • Developer: One person implementing changes from an issue
  • Reviewer: One or more persons reviewing the changes

Developer-Guide pt. I: Create a Pull Request for your Changes

For quick reference, see condensed checklist

On the naming of branches

Please check out CONTRIBUTING.md for naming conventions and good Git commit messages.

Create draft pull-request

Start condition: You've completed your content edits of the ontology.

  1. 🐙 Push your local git-branch to the OEO-repository.

  2. 🐙 Create a draft pull request

  • select your branch as compare

  • select dev as base

  1. 🐙 Fill pull-request template

  • Provide a meaningful title (short and imperative)

  • Summarize changes in a self explanatory way
    A summary of the discussions and conclusions from associated issues, makes reviewing easier. Ideally, the description provides all necessary information, so that reviewers do not need to read the hole discussion(s) of the associated issue(s).

  1. 🐙 Reference related issues
    Use closes #IssueNumber in the title or description to link and automatically close related issues on merging. This keeps the issue-section tidy.
    Quick-Tip: You might also use other special keywords which suit your style of writing.

  2. 🐙 Assign yourself as assignee for the PR

Document your content changes

Start condition: You've created a draft pull request, so that you can obtain a PR-number.

In the following you'll need the PR-number which is shown next to your PR's title:

image

  1. 📝 Update CHANGELOG.md
    Make sure to always reference your PR-number and pay attention to the conventions described in the CHANGELOG.md.

  2. 📙 Include term-tracker annotation in ontology
    For details concerning the term-tracker, see term tracker annotation

    • for new classes: add the term tracker annotation to class definition

    • for updated classes: append your new information as described in term tracker annotation

    • for individual axioms: Normally attaching annotation directly to axioms is possible, BUT there's a bug currently.
      Please update the term tracker annotation to the class definition in the mean time.

  3. 🔶 Stage, commit and push the new changes originating from including the term tracker information.

Publish your PR

Start condition: All changes originating from documentation are pushed to 🐙

  1. 🐙 Make sure that all CI-unit-tests passed
    The CI-tests ensure that there are no fundamental errors within the changes.
    In case they fail and the error message is cryptic, don't hesitate to ask for help!

    ⚠: Only proceed, if the CI-test have been successful and there are no inconsistencies!
    In case of inconsistencies, try to find the core problem e.g. via Protégé's "Explain"-functionality.
    Make sure that a potential fix, neither is hiding symptoms only nor is breaking the functionality which shall be implemented.
    New features sometimes trigger unrelated, hidden bugs which have been sleeping in the code-base for a long time!

  2. 🐙 Ask for review of your PR by assigning reviewers
    Assign 1-2 participants of the issue-discussion as reviewers.

    Reviews ensure that implementations reflect the aspects agreed on during discusssion.
    If the review requires special expertise, contact a dedicated team-members.
    See the team-tag section in README.md which contains more information about the expertise of different team members.

  3. 🐙 Press "Ready for review"-button
    This will publish your PR. 🎉

  4. After the review you take responsability for merging your proposed changes. Read Developer-Guide pt. II for instructions

Developer-Guide pt. II: Merge your Changes

Remember that also review require effort, so show recognition for the review performed.

  1. 🐙 Merge pull request

  2. 🐙 Delete branch

  3. 🐙 Close issues that have not been part of automation (e.g. Meta-Issues)

Reviewer-Guide: Check Changes Introduced by a Pull-Request

For quick reference, see condensed checklist 🏷TODO/ADD_LINK

The OEO-development spans multiple areas of expertise (domain knowledge, git functionalities, ontology specifications, etc.).

Not every reviewer can comment on every aspect of a PR, but every contribution to a review is welcome.
So, don't be afraid to add a partial review mentioning that it only spans your field of expertise.

  1. 📙 or 📝 Review the content of changes
    Depending on the nature of changes, there are different things to consider.
    Here are some questions to ask yourself when reviewing concerning ...

    • Are classes and axioms placed correctly in the hierarchy?
    • Are spelling and grammar of definitions as well as axioms correct?
    • Are class definitions also reflected in the class axioms?
    • Are their hidden interactions in the ontology?
    • Are classes sorted correctly according to the BFO?
    • Are changes placed in the correct oeo-module?
    • Are all steps of specific changes done?
      (e.g. update of class definition when moving below another parent class
      🢂 see implementation specific checklists for guidance
    • Are all changes intentional ones? (e.g.Sometimes Protégé rearranges things which goes wrong in rare cases)
    • Are there only changes that were previously agreed on in the discussion?

  2. 📙 or 📝 Check for term tracker annotation
    There shall be an annotation for each changed class in the class description
    ⚠: Make sure that the additions don't overwrite older information.

  3. 🐙 Check CHANGELOG.md
    Make sure that changes are noted down

    • in the right section ("added"|"changed"|"deleted")
    • include the PR-number
      AND
    • have an spelling identical to their rdfs:label in the ontology

  4. 📙 If there are changes in more than one of the oeo-modules (i.e. *.omn files), please check in Protégé for each module, if there are "runaway" classes. A "runaway" is a class that shows up in the module only with its identifier and looks like this:
    grafik
    If so, ask the developer to move the affected class to the oeo-shared module. Please note, that moving one class can lead to new "runaway" classes, so sometimes a couple of classes need to be moved.

  5. 🐙 Approve, comment or request changes
    Depending on the previous steps provide your feedback, propose changes and work together with the pull-request author on fixing potential problems.

    • Small typos might be corrected directly by the reviewer.
    • Directly annotating sections/lines keeps the discussion structured.
    • Fundamental problems shall not be discussed in the PR.
      If things have to be re-thought, push the discussion back to the related 🐙-issue

You may use the commenting functionality: grafik

Remember that every contribution requires effort, so show appreciation for the work done.

This page is based on issue #1104

You already know what to do? Perfect, make sure that you miss no step by using checklists

Symbols used in this wiki:

🐙➔ github | 🔶➔ git | 📙➔ protégé | 📝➔ text editor | 🤖➔ robot

Clone this wiki locally