Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CONTRIBUTING.md: Schema development process, improved mermaid diagram, etc. #4186

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

handrews
Copy link
Member

More work towards #3715 and #3716.
Also paging @jeremyfiel as GitHub won't let me add you as a reviewer.

Copy link
Contributor

@jeremyfiel jeremyfiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear and concise. Well done!

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, minor nits

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ralfhandl ralfhandl requested a review from a team November 11, 2024 10:40
@handrews
Copy link
Member Author

@ralfhandl I have made some changes and updated. Changes include:

  • Consistently showing the v prefix on the branches (apparently I couldn't make up my mind on this, but the diagram matched the branches I created so "with v" it is)
  • Split "Development" and "Publishing" into separate sections at the same level, which hopefully makes it clear that schema development is resolved, but schema publishing (to the best of my knowledge) is not, and should be addressed in a separate PR rather than blocking this one
  • Reworked the branching / merging steps to streamline them and include examples of some of the more hard-to-follow steps (this should make it clear that the "only" in the bit about merging patch releases to dev meant "only merge to dev if these conditions are met" and not "merge onlyt to dev, not main", as I just now figured out that was probably the source of confusion)

@ralfhandl
Copy link
Contributor

@handrews Why do we need the dev branch?

If we would branch vX.Y+1 from the same commit in vX.Y where vX.Y.0-rel was branched off, we would get the same commit history, and we would save the effort of updating the dev branch from two release lines. Changes would flow from the "current minor" branch to the right into the "next minor" branch, and only from *-rel branches to the left into main.

What would we loose with this simplification?

@lornajane
Copy link
Contributor

We needed the dev branch when switching branching setups to give us a place to rename the files before branching. I believe that at this point, the branching process is very clearly laid out. Let's follow it as it is and then see what amendments or optimisations we might use.

lornajane
lornajane previously approved these changes Nov 13, 2024
@lornajane lornajane requested a review from a team November 13, 2024 09:20
@ralfhandl
Copy link
Contributor

We needed the dev branch when switching branching setups to give us a place to rename the files before branching.

That is my impression: the dev branch was temporarily necessary and can be removed from the picture without loosing anything, and significantly simplifying the picture.

handrews and others added 5 commits November 13, 2024 07:59
From later comments on the previous update.  This shows a more
comprehensive example, including  merging patch releases from
the most recent line _only_ back to `dev`.
And also add some section headers and clarify publishing a bit.
Co-authored-by: Ralf Handl <[email protected]>
@handrews
Copy link
Member Author

@ralfhandl we have had this discussion before.

I just added a section explaining further. I don't like just stacking vX.Y-dev branches on each other endlessly. dev essentially replaces main for anything that doesn't need to be fully publicly published.

(BTW, we could dispense with the vX.Y.Z-rel branches, but it would involve doing weird things, namely doing a git mv and then reverting half of it, and I'd rather just use git normally as much as possible. Having that vX.Y-Z-rel rename commit that is not on vX.Y-dev also gives us a better place for any future "wtf wy did the IETF move thier RFCs again" link fixes that we might need to do one day.)

@ralfhandl ralfhandl requested review from lornajane and a team November 14, 2024 07:55
Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @handrews!

@lornajane lornajane merged commit f3161e5 into OAI:main Nov 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants