Release 2.7.0 #642

ravage84 · 2019-07-28T17:15:36Z

We are finally ready to release PHPMD 2.7.0! 🚀

This release contains a lot changes, actually over 250 commits.
The release, which includes new features, improvements & fixes is long overdue.

It really needs to get out of the window.
Nonetheless, it should not be rushed.
I targeted the actual merge of this and the tag & GH release for tomorrow, 29.07.2019.

I kindly ask you to review this thoroughly.

For this, I created a check list:

All relevant pull requests and commits for 2.7.0 are included in changes.xml
All the 2.7.0 change entries in changes.xml have the correct date (commit hash)
All the 2.7.0 change entries in changes.xml have the correct developer
All the 2.7.0 change entries in changes.xml have the correct type of change
All the 2.7.0 change entries in changes.xml have the correct change system (all github)
All the 2.7.0 change entries in changes.xml have the correct issue number
All the 2.7.0 change entries in changes.xml have have a understandable change text
All the 2.7.0 change entries in changes.xml have the type of due to mention, if relevant

Please, take note that you cannot simply use the comparison between 2.6.0 and master, as it only shows the 250 newest changes. For the first few commits, you have to use the commit history starting after the last commit for 2.6.0.

Please state which checks you performed in your review.

kylekatarnls

I did not review all the changes in case we decide to reduce the list. If not I will continue.

My point of view is:

Implement keyword should be reserved for actual new features of the library.

The CHANGELOG should help the user to see what's new. If he want a technical history of the repository, he/she can use the GIT history.

Our CHANGELOG and release notes should mention only new features and features changes in the public API (and eventually protected scope).

That should not include in my opinion:

Code style (if no risk of behavior change)
Settings changes of tooling (Travis, PHPUnit, Scrutinizer)
Documentation or any part that are not in the final downloaded package
Comments (except eventually for PHPDoc of public methods/property/classes)
Typos (change of wording in descriptions, in --help, options explanation)

The CHANGELOG should mention initial commits rather than merge commit whenever possible to keep the authorship.

Each change should appear only once in the CHANGELOG.

We should add a BC notice, in this release we have minimum > maximum as a break change. Even if it was an obvious mistake, users may have started to use it and will need to correct their settings. A small explanation: "We're aware it's a BC and apologize for the disturbance, we couldn't reasonably upgrade to a major release and had to make a pragmatic choice regarding the renaming of this option."

CHANGELOG

tvbeek · 2019-07-29T11:16:22Z

I agree with @kylekatarnls that the changelog should contain the changes for the user and not the other changes (code style, typos in documentation, ...)

Maybe it is also an idea to have some headers between the different parts:

Added:

...
...

Fixed

...
...

ravage84 · 2019-07-29T13:01:37Z

Thank you for your feedback. I guess I should have explained myself better. Instead of doing what I personally would prefer to do, I went and did what we have done in the past.

I'm not sure what we included and what we excluded, so I went through all the commits and tried to include (almost) everything. At least I wanted to add each and every PR that went in.

@kylekatarnls

Implement keyword should be reserved for actual new features of the library.

This is word is auto-generated through the Ant target. Like I said, that's the way it used to be.
Please mostly ignore the change log, focus on the changes.xml file, instead.

The CHANGELOG should help the user to see what's new. If he want a technical history of the repository, he/she can use the GIT history.

I agree on that in principial. For me there are three or four distinct things:

Commit history
Issue tracking (including PRs)
Change log
Release notes

And yes, the change log should not be a 1:1 copy of any of the other two, nor a full combined list.

Our CHANGELOG and release notes should mention only new features and features changes in the public API (and eventually protected scope).

That should not include in my opinion:
* Code style (if no risk of behavior change)
* Settings changes of tooling (Travis, PHPUnit, Scrutinizer)
* Documentation or any part that are not in the final downloaded package
* Comments (except eventually for PHPDoc of public methods/property/classes)
* Typos (change of wording in descriptions, in --help, options explanation)

It should contain a list of things that could be relevant for the maintainers, contributors and/or end developers, whatever it is. And website buildng & documentation changes could be relevant for some of them in some cases.

The release notes can be trimmed down to the most relevant and link to the change log. This way the visibility is the greatest for the most important stuff but people can easily learn more.

The CHANGELOG should mention initial commits rather than merge commit whenever possible to keep the authorship.

Well, personally, I wouldn't do that in the future, since we merge through PRs, we can simply link to the related PR(s).
I'm not 100 % sure how it was handled in the past if a change was spead throughout several commits, but for me iit's better to see the full change at once. I don't want nor need to follow every commit. The end result is what is actually included in the code I get with a given release.

Each change should appear only once in the CHANGELOG.

Agreed. If something is mentioned twice, it's just my fault.

We should add a BC notice, in this release we have minimum > maximum as a break change. Even if it was an obvious mistake, users may have started to use it and will need to correct their settings. A small explanation: "We're aware it's a BC and apologize for the disturbance, we couldn't reasonably upgrade to a major release and had to make a pragmatic choice regarding the renaming of this option."

Great idea. Thanks for bringing this up. I will include it in the GitHub releae text, as it is most visible there. But we can change the current text of the related text to better show what has changed, too.

@tvbeek

I agree with @kylekatarnls that the changelog should contain the changes for the user and not the other changes (code style, typos in documentation, ...)

See my explanations above.

Maybe it is also an idea to have some headers between the different parts:

In the future, I'd like to adopt Keep a Changelog, as previously discussed.

ravage84 · 2019-07-29T13:03:38Z

With that explained, please let's review the changes.xml.
If something should be removed because it's duplicated or not relevant enough, tell me.
Le'ts ignore the way the current change log is worded and how it looks. We can improve on that in the future. One step at a time. I want to get this important release out of the window.

ravage84 · 2019-07-29T21:35:49Z

Two reasons on why I haven't removed all the "unimportant" changes:

This is the culmination of two and a half years of accumulated changes by many contributors. Now, we finally release a version. Let's relfect the fact that there were over 250 commits since the last minor version.
Attributing the contributors of these changes in our changelog through a link to their PR can be motivating. Let's mention all these changes, even if small.

By the way in the release notes, I've planned to only mention the most important ones and link to the change log. Interested parties can follow that link.

S, please, let's review this and get it flying.

tvbeek · 2019-07-29T21:59:33Z

It is a big list to review, so for me it will take some time.
Did you generate the changes.xml with a tool or is it a manual job?

ravage84 · 2019-07-29T22:06:55Z

It is a big list to review, so for me it will take some time.

Sure, take your time. A day or two more won't hurt after 2 and a half years 😼

Did you generate the changes.xml with a tool or is it a manual job?

Nope, it's "hand-made" but the CHANGELOG is generated based on the changes.xml by a Ant target.
So, you can mostly ignore that.

src/site/docx/changes.xml

kylekatarnls

I checked authors/PR numbers/commits/numbers and quickly change title

My commits number changes:
a073c06
1d182c5

My wording changes:
5c9cbde
dd1c33b
c031c98
91da00e

ravage84

Thank you for your changes, @kylekatarnls
Please, have a look at my comments.

src/site/docx/changes.xml

ravage84 · 2019-07-30T12:28:15Z

Now, this needs to wait for #643 and then we should include the change entry just below the original bump change.
Once that is done, it just needs two approvals and we are good to go.

- Add reminder for BC breaking change in release intro - Add PHP bump in release intro - Reorder changes to (more or less) reflect their importance (new features first, then improvements & fixes, later dependency & build changes, style & documentation last) - Remove duplicated entries - Improve wording of entries

As Manuel doesn't actively maintain it anymore. People shouldn't bother him, they can write us, the team which is now behind PHPMD.

There is only one execution priority.

Makes clear it's a CI. Name is without dash, appartently.

ravage84 added Documentation Website labels Jul 28, 2019

ravage84 added this to the 2.7.0 milestone Jul 28, 2019

kylekatarnls reviewed Jul 28, 2019

View reviewed changes

ravage84 requested review from jakzal, JeroenDeDauw, kylekatarnls, MarkVaughn, tvbeek and eeree July 29, 2019 14:10

ravage84 force-pushed the release-2.7.0 branch from e0756fb to 760a57d Compare July 29, 2019 21:25

kylekatarnls reviewed Jul 30, 2019

View reviewed changes

src/site/docx/changes.xml Show resolved Hide resolved

kylekatarnls previously approved these changes Jul 30, 2019

View reviewed changes

ravage84 commented Jul 30, 2019

View reviewed changes

src/site/docx/changes.xml Outdated Show resolved Hide resolved

src/site/docx/changes.xml Outdated Show resolved Hide resolved

src/site/docx/changes.xml Outdated Show resolved Hide resolved

src/site/docx/changes.xml Outdated Show resolved Hide resolved

kylekatarnls dismissed their stale review via 7314cd3 July 30, 2019 12:21

kylekatarnls previously approved these changes Jul 30, 2019

View reviewed changes

kylekatarnls dismissed their stale review via fa070cb July 30, 2019 12:24

ravage84 and others added 6 commits July 30, 2019 14:53

Update XML change log for release 2.7.0

8573cdc

Bump project version to 2.7.0

9615f55

Update change log for release 2.7.0

420c74e

Fix typo

9b8b3d2

Fix CHANGELOG time concordance

fd0eb3c

kylekatarnls added 7 commits July 30, 2019 14:53

Fix changes types in CHANGELOG

89b0fcd

Fix verbs

11fefdb

Fix commits swap

e9f6958

Fix commit of #383 better explanation

2362f04

Fix changes wording

f0dfb51

Fix changes wording

15387f5

Add #643 to the CHANGELOG

15aecae

kylekatarnls force-pushed the release-2.7.0 branch from fa070cb to 15aecae Compare July 30, 2019 12:59

ravage84 added 13 commits July 30, 2019 15:07

Change author to "PHPMD maintainer team", and use general email address

6e79368

As Manuel doesn't actively maintain it anymore. People shouldn't bother him, they can write us, the team which is now behind PHPMD.

Change release date back to today

3e0fb99

Use simpler language

3d07fe2

a -> the

afca04e

There is only one execution priority.

Add note about BC break

2c0fe9d

Do not use abbreviated word

182c9a2

Improve wording

d18a7c5

Use "Travis-CI" everywhere

98aee24

Use "Scrutinizer CI"

4b02536

Makes clear it's a CI. Name is without dash, appartently.

Use "AppVeyor CI" everywhere

0b2e7dc

Makes clear it's a CI. Name is without dash, appartently.

Remove double space

7d79dff

Improve wording

fcc1e32

Update CHANGELOG based on changes.xml for PHPMD 2.7.0

5eff9e6

kylekatarnls approved these changes Jul 30, 2019

View reviewed changes

tvbeek approved these changes Jul 30, 2019

View reviewed changes

hbaeumer approved these changes Jul 30, 2019

View reviewed changes

kylekatarnls merged commit a05a999 into master Jul 30, 2019

ravage84 deleted the release-2.7.0 branch July 31, 2019 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release 2.7.0 #642

Release 2.7.0 #642

ravage84 commented Jul 28, 2019 •

edited by kylekatarnls

kylekatarnls left a comment •

edited

tvbeek commented Jul 29, 2019

ravage84 commented Jul 29, 2019

ravage84 commented Jul 29, 2019

ravage84 commented Jul 29, 2019

tvbeek commented Jul 29, 2019

ravage84 commented Jul 29, 2019

kylekatarnls left a comment

ravage84 left a comment

ravage84 commented Jul 30, 2019

Release 2.7.0 #642

Release 2.7.0 #642

Conversation

ravage84 commented Jul 28, 2019 • edited by kylekatarnls

kylekatarnls left a comment • edited

Choose a reason for hiding this comment

tvbeek commented Jul 29, 2019

Added:

Fixed

ravage84 commented Jul 29, 2019

ravage84 commented Jul 29, 2019

ravage84 commented Jul 29, 2019

tvbeek commented Jul 29, 2019

ravage84 commented Jul 29, 2019

kylekatarnls left a comment

Choose a reason for hiding this comment

ravage84 left a comment

Choose a reason for hiding this comment

ravage84 commented Jul 30, 2019

ravage84 commented Jul 28, 2019 •

edited by kylekatarnls

kylekatarnls left a comment •

edited