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.

Sign up for GitHub

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

Jump to bottom

Update governance guidelines #105

Merged
merged 6 commits into from
Jul 4, 2021
Merged

Update governance guidelines #105

merged 6 commits into from
Jul 4, 2021

Conversation

waldyrious
Copy link
Collaborator

@waldyrious waldyrious commented Jul 4, 2021 •
edited
Loading

See the relevant discussions in #102 for context.

waldyrious
waldyrious commented Jul 4, 2021
View reviewed changes
GOVERNANCE.md Outdated
@@ -33,12 +35,13 @@ A PR should be created with all relevant changes to update the repository for th
It should apply the following actions:

1. Change the "Unreleased version" heading in the CHANGELOG.md file
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Xymph any thoughts about renaming this to "Upcoming version" or "Next version", as I suggested here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Upcoming version" works for me as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 1a9ae64 :)

waldyrious
waldyrious commented Jul 4, 2021
View reviewed changes
GOVERNANCE.md
@@ -33,12 +35,13 @@ A PR should be created with all relevant changes to update the repository for th
It should apply the following actions:

1. Change the "Unreleased version" heading in the CHANGELOG.md file
to the appropriate version name (e.g. "Version 1.2.3")
and add a new "Unreleased version" section heading above it;
to the appropriate version name and date (e.g. "Version 1.2.3 - 2020-12-31")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe it would make sense to replicate the changes from e2c026c in this PR, so that they're introduced in lockstep with these guideline changes. WDYT @Xymph?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not really biased either way, but if you are splitting off commits from #102 anyway, I'm fine with that commit going into this PR too. Along with the initial header change per my comment above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't cleanly cherry-pick that commit into this branch, because it includes adding the section header for the new release (which should remain in #102), but I'll recreate the parts that need to be moved and will mark you as co-author of the commit :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine, if I need to make changes to my branch locally, and pull/push stuff around, please detail any steps.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, no worries. In fact, I should be able to update your branch myself. Then, if it looks good to you on GitHub, you will be able to pull the updated branch to your local clone as well.

@waldyrious waldyrious mentioned this pull request Jul 4, 2021
Merged
@Xymph
Copy link
Collaborator

Xymph commented Jul 4, 2021

No further comments on the committed changes, they look fine to me.

@waldyrious waldyrious force-pushed the update-governance branch from c928b02 to e6735a2 Compare July 4, 2021 18:41
@waldyrious
Copy link
Collaborator Author

waldyrious commented Jul 4, 2021 •
edited
Loading

OK, changes made. Please check the updated commits. I've updated the PR description as well.

@Xymph
Copy link
Collaborator

Xymph commented Jul 4, 2021

Almost everything looks fine to me, the only previous change I don't see anymore is the formatting of Wikimate.php in bullet 3 of the last section.

Btw, remember to add #105 to the Changelog for v0.13.0.

@waldyrious
Copy link
Collaborator Author

waldyrious commented Jul 4, 2021 •
edited
Loading

The formatting reversal was intentional — the other filenames in that document are not code-formatted, so I reverted it for consistency. I can change them all to be code-formatted, though.

As for adding a changelog entry — OMG, of course! I'll add a new commit for that.

Xymph
Xymph approved these changes Jul 4, 2021
View reviewed changes
Copy link
Collaborator

@Xymph Xymph left a comment •
edited
Loading

Choose a reason for hiding this comment

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

16c8d28 looking good now.

Xymph
Xymph reviewed Jul 4, 2021
View reviewed changes
Copy link
Collaborator

@Xymph Xymph left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Re. f3d4338: The Governance doc is a new addition in v0.13.0, so instead of a separate entry under Changes, wouldn't it be logical to simply add this PR to that first entry, like "(#83, #105)"? Similar to older changelog entries combining multiple PRs.

@waldyrious
Copy link
Collaborator Author

Good point. I forgot we could combine multiple PRs into the same entry. Will change.

@waldyrious waldyrious force-pushed the update-governance branch from 16c8d28 to 2e771cd Compare July 4, 2021 20:16
@Xymph
Copy link
Collaborator

Xymph commented Jul 4, 2021

Great, looks ready to wrap up.

@waldyrious
Copy link
Collaborator Author

Great :) Please go ahead at your convenience.

@Xymph Xymph merged commit 12f25bc into master Jul 4, 2021
@waldyrious waldyrious deleted the update-governance branch July 4, 2021 20:54
Xymph
Xymph reviewed Jul 5, 2021
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xymph Xymph Xymph approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@waldyrious @Xymph