slonopotamus
commented
3 years ago Thus far, I've been using
resolves #<issue number> summary goes here (PR #<pr number>).
Please, please, omit that PR. I am just too lazy and forget every time to type PR. GitHub itself autosuggests just (#number), without PR part.
This makes it easier to organize branches locally
I don't think conventions should care how user organises their local branches? Also, most likely external contributors won't follow branch naming conventions. Will you reject their PRs just because they named the branch unconventionally? Note that it isn't possible to change branch name without recreating PR.
All the rest: I'm +1 about resolves <issue number> subject line, changelog entries, issue requirement and merge strategy that you use (rebase'n'squash).
mojavelinux
abelsromero
ggrossetie
Within the Asciidoctor community of projects, I think we should agree on a general set of conventions / guidelines for PRs (at the very least, for the core projects and those closely associated with it). This will make it easier to work in the various projects without having to remember what all the individual rules are. I have been adhering certain conventions over the years, but even those may need to be revisited and revised. What I'm about to propose are meant to be general guidelines, not strict rules. But the closer we stick to the guidelines, the easier it is to get into the flow.
issue requirement - First, I think it should be a recommendation that every human-created PR has an associated issue. The idea is that the issue is where the decision is made to approve the proposed idea and proceed with a code fix before anyone has invested time working on it. It's also where the architecture for the issue can be discussed. The issue number is what gets referenced by the CHANGELOG, so we want that to point to that initial conversation / full context. The PR can then be focused on the code review itself.
branch name - Second, The branch from which the PR is submitted should include the issue number and a short description in the form issue-\-\ (e.g., issue-1897-keep-anchor-with-block). This makes it easier to organize branches locally and ensures we don't get PRs from branches that are changing while we are trying to review the PR.
subject line - Third, we should agree on the format for the subject line for a PR. Should it start with an action keyword, like fixes|resolves|closes \? If we use these prefixes, when it's merged, the issue automatically gets closed. I know that GitHub refuses to recognize the issue number in the subject. But if it's not in the subject, then you can't quickly scan for issue-closing commits in the git history after merging it. So perhaps we need it in both the subject and the description.
changelog entry - Fourth, every PR should include an addition to the CHANGELOG. I know some people like to generate a CHANGELOG at the time of a release, but then no one knows what's in a release until it's made. Keeping the CHANGELOG up to date with each merge has proved to be very effective in communicating what's coming. It's a living document. It also makes doing the release a lot simpler.
merge strategy - Finally, how do we merge and what template do we use for the subject? Thus far, I've been using
resolves #<issue number> summary goes here (PR #<pr number>). Does that seem right? Here's how it looks: https://github.com/asciidoctor/asciidoctor/commit/a1505efca095aa6a8ab33fb34a0937cd26cbe822Of course, we can't mandate behavior. That's not the point. The point is for us maintainers to lead by example and develop a workflow that's consistent and works well across the projects. What other recommendations would you add that have proven to work well?