-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Write up NEWS rules #19387
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
base: master
Are you sure you want to change the base?
Write up NEWS rules #19387
Conversation
ed389b1
to
2e8ac0f
Compare
2e8ac0f
to
09f111e
Compare
09f111e
to
ad25e20
Compare
*Bug fixes*: In the lowest supported branch, and in each newer branch, except | ||
for master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably clarify that master
should be updated the moment alphas are tagged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this true only for things that were introduced in master, i.e. fixed specifically for master?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I am aware? I have always added bug fixes to the NEWS file in master after alpha releases so that people can know if something was fixed in alpha 2, see: eaf24ba
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not done this, and as (I thought) I was updating current practise, I didn't add that. I always thought that we would add them in NEWS as soon as php-next was branched though.
I don't particularly care which way we decide this.
*Feature additions*: In master only. | ||
|
||
If for some reason a feature is introduced in a branch lower than master (this | ||
isn't strictly allowed), then the entry must also be in master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we even done this properly for 8.5?
What about deprecations, should those also be in NEWS?
I feel the process we had at one point where NEWS
was just for bug fixes and new features, deprecations, BC breaks, etc. where in UPGRADING to be way more clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was always told that NEWS is a changelog, so every observable change needs to go there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extreme example: if we only have new features between two alphas, and no fixes, with your rule we'd have an empty NEWS which doesn't make sense to me as there were changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we even done this properly for 8.5?
If you mean the "a feature introduced in branch lower than master", then, I'm not sure. Because I don't believe we have recently added features to the PHP-8.4
branch.
What about deprecations, should those also be in NEWS?
Deprecations also ought to be in NEWS, IMO. Like @nielsdos says, "NEWS is a changelog, so every observable change needs to go there".
But the paragraph that you replied to would currently apply to the PHP-8.4
(and earlier branches). Our policies don't allow new features in that branch, but it has happened in the past.
NEWS should contain simple line items what has been added/removed/changes, with UPGRADING describing in more detail how you would go about replacing removed/changed features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any new features are explicitly prohibited in the policy so this should not be here as we should never add new features. It might just bring confusion that there is a chance to add them - at least it should be this is strictly prohibited
instead of this isn't strictly allowed
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also clarify UPGRADING, and whether new features must go into both NEWS and UPGRADING? This is still handled inconsistently. I usually only add it to UPGRADING, but some people do both.
When you change the NEWS file for a bug fix, then please keep the bugs sorted in | ||
decreasing order under the fixed version. | ||
When you change the NEWS file for a bug fix, then please keep the bugs sorted | ||
under the fixed version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this hints at the fact that the patch notes of multiple versions live in the same NEWS file, but it makes it sound like the list itself must be ordered in some way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it sounds quite confusing...
Fixed bug GH-{number} ({issue-description}). ({contributor}) | ||
|
||
Bug fix entries SHOULD be clustered together, and ordered according to their | ||
issue number. The `{issue-description}` SHOULD represent what the actual bug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I mind, but I've never seen entries being listed by issue number, and I see no value-add for this. Usually people just append to the bottom of the list. Maybe that's what the text I previously commented on referred to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a rule that has been relaxed in the last decade, but we used to be very deliberate doing it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I was unaware of that, seems rather useless. But I can adapt.
a non-Generator delegate crashes). (Arnaud) | ||
|
||
- OPcache: | ||
. Make OPcache non-optional (Arnaud, timwolla) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. Make OPcache non-optional (Arnaud, timwolla) | |
. Make OPcache non-optional. (Arnaud, timwolla) |
*Bug fixes*: In the lowest supported branch, and in each newer branch, except | ||
for master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this true only for things that were introduced in master, i.e. fixed specifically for master?
Btw, isn't this something that should live in https://github.com/php/policies/? |
To answer all the other single-comment comments:
This was about NEWS, and new features should be in there. It's a full overview of what is in the release. We can tackle UPGRADING in a future patch. I can also add a section on UPGRADING. For what I understood that should contain all the information that you need for upgrading between versions. So it should document changes, deprecations (with their alternatives), etc. I don't think it needs to contain new features, but that doesn't seem to be how it is currently used.
I don't understand what you're trying to say here.
Maybe? It was always here, and I didn't see a reasonable location in policies for this. |
Disregard, I wasn't aware the order-bugs-by-id rule was a thing. |
Depending on what sort of fix it is, or where in the release cycle we are, | ||
different NEWS files must be updated. | ||
|
||
*Security fixes*: In the lowest security-supported branch, and in each newer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a minor detial that from time to time some security fixes are only present in later versions.
|
||
If an entry pertains a bug fix, they MUST be formatted as: | ||
|
||
Fixed bug GH-{number} ({issue-description}). ({contributor}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should still keep the bug tracker format as bugs are still being fixed there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean in addition to the GH one, of course.
This addition documents how to update NEWS in one place, instead of it being on the wiki, here, and scattered throughout documents.
I believe that I have documented the current practise, but comments welcome.