Skip to content

Conversation

@TrialDragon
Copy link
Member

Objective

Centralize, and add back missing, PR merging rules to the Contributing Guide.

Solution

Put the four merging rules in the Reviewing Pull Requests page under How Pull Requests are merged. Deduplicate partial mentions of these rules, and instead direct to this section as a single source of truth.

Testing

Run zola serve --open on the trialdragon/clarify_pr_merge_rules branch, and check that the changed areas render correctly, and that Zola does indeed run.

@alice-i-cecile
Copy link
Member

@lewiszlw could you take a look at this one too? :)

Copy link
Member

@lewiszlw lewiszlw left a comment

Choose a reason for hiding this comment

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

The content looks good to me. But I didn't find the original text of How Pull Requests are merged in main repo.

@TrialDragon
Copy link
Member Author

The content looks good to me. But I didn't find the original text of How Pull Requests are merged in main repo.

The original text was in docs/the_bevy_organization.md under the Maintainer header. I moved it to Reviewing Pull Requests here since it is useful info for plenty of contributors, and could get lost or forgotten in its general original place.

@TrialDragon TrialDragon added S-Ready-For-Final-Review Ready for a maintainer to consider for merging and removed S-Needs-Review labels Aug 23, 2024
Copy link
Contributor

@mweatherley mweatherley left a comment

Choose a reason for hiding this comment

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

Just some grammar nitpicks. Looks good otherwise.

Co-authored-by: Matty <[email protected]>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 23, 2024
Merged via the queue into bevyengine:main with commit a97b158 Aug 23, 2024
@TrialDragon TrialDragon deleted the trialdragon/clarify_pr_merge_rules branch August 23, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants