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

Ensure that all references to creating and reviewing PRs mention blurb and NEWS entries #358

Open
ned-deily opened this issue Mar 31, 2018 · 3 comments

Comments

@ned-deily
Copy link
Member

Other than for the most trivial sorts of changes (e.g. obvious spelling errors), nearly all changes to the cpython repo should include a NEWS entry; that's our primary mechanism to communicate changes with downstream users. We should be encouraging everyone, including non core-developers to submit PRs with NEWS entries. Among other benefits, having a NEWS entry makes reviews of the PR easier and helps to focus everyone include the submitter on what problem the PR is trying to address. And the tool we have to create NEWS entry is blurb; we want to encourage everyone creating or modifying a PR to use it. It's easier to delete the occasional unnecessary NEWS item than it is to create one when committing. To that end, suggest:

  1. Create a section in the Git Bootcamp and Cheat Sheet Accepting and Merging section to describe the details of using blurb with a fallback to how to manually create a MiscNEWS.d entry: basically, adapt and move the detailed info from the Accepting Pull Requests section. We want to be careful to encourage blurb usage and not encourage manually creation as the details could change in the future.

  2. This section should also include some guidelines on what a good NEWS entry looks like, or, if not here, one place in the devguide with references to it.

  3. Review the whole document and add mentions of NEWS entries and the blurb command everywhere creating or reviewing or committing a PR is discussed and include a link to the detailed section created above. In particular, the Lifecycle of a Pull Request section needs to be edited to include blurb.

@ned-deily
Copy link
Member Author

A small but important point: in the past, i.e. before we had blurb, managing NEWS entries was a small nightmare because of the merge conflicts inherent with multiple PRs attempting to edit the same monolithic NEWS file. So we previously discouraged creating NEWS entries in patches and early PRs to avoid these merge conflicts. blurb was created to solve that problem; now each NEWS entry is a separate file, eliminating NEWS merge conflicts. Thus the devguide needs to be updated to recognize that huge change. The NEWS is good!

@manueljacob
Copy link
Contributor

I agree that this should be clarified in the devguide. On my first PR, I was irritated (that word is maybe too strong) that the "bedevere/news" check fails. The devguide is not very clear on whether I should add the NEWS entry or the person accepting the PR should do it.

  • This checklist doesn’t mention it, but it is mentioned that patchcheck checks it.
  • This suggests that usually the core developer should do it, but maybe I should do it.

@terryjreedy
Copy link
Member

There are +s and -s with OP including a blurb. I plus is that the PR cannot be accidentally merged without some action. But on balance, I think it should be recommended whenever there is a bpo number. Before merging, I can edit or even delete, and editing online is easy.

@willingc willingc added guide-edits Editing of doc or content needed topic-pull requests enhancement labels Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants