Skip to content

Changelogger: try a new format #21140

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
Merged
samiff merged 16 commits into from
Sep 24, 2021
Merged

Changelogger: try a new format #21140

samiff merged 16 commits into from
Sep 24, 2021

Conversation

@jeherve
Copy link
Member

@jeherve jeherve commented Sep 21, 2021 •
edited
Loading

See #21138

Changes proposed in this Pull Request:

  • We're introducing a new possible format for Jetpack plugin releases, so the Changelogger must support it. The format will be 2109.1, i.e. 2021 September, first Atomic release.

Jetpack product discussion

  • p9dueE-3ca-p2

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

  • Do the tests pass?

@jeherve jeherve self-assigned this Sep 21, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 21, 2021 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ⚠️ All commits were linted before commit.
  • 🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

anomiex
anomiex reviewed Sep 21, 2021
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

You'll also need to add some test cases to the provideCompareVersions method. Be sure to compare with things like "10.1-2109.2" versus "10.1-2109.10" in addition to the more obvious things.

You might also add some checks in provideNormalizeVersion for invalid versions, like "10.1-2113.1" (there's no 13th month) or "10.1-2100.1" (no zeroth month either).

@anomiex
Copy link
Contributor

anomiex commented Sep 21, 2021

Also add tests that compare like "10.1-2109.1" with things like "10.1-alpha" and "10.1-beta" to make sure the ordering there is correct too.

jeherve and others added 2 commits September 21, 2021 17:09
@jeherve @anomiex
Update projects/packages/changelogger/src/Plugins/WordpressVersioning…
e1fcaf4 
….php

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@anomiex @jeherve
Add more test cases
fc734f5
@jeherve jeherve mentioned this pull request Sep 21, 2021
Closed
@jeherve jeherve added [Pri] Normal [Status] Needs Review This PR is ready for review. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] In Progress labels Sep 21, 2021
jeherve added a commit that referenced this pull request Sep 21, 2021
@jeherve
Updateversion number again
1e88ea4 
Based on discussion in #21140
@anomiex anomiex added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Sep 21, 2021
@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Sep 22, 2021
@samiff samiff requested a review from anomiex September 22, 2021 16:45
anomiex
anomiex reviewed Sep 23, 2021
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

Some suggestions inline.

@anomiex anomiex added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Sep 23, 2021
anomiex
anomiex previously approved these changes Sep 24, 2021
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

Looks good to me. Needs a master merge though.

@samiff samiff merged commit 1134f00 into master Sep 24, 2021
@samiff samiff deleted the update/changelogger-version-wordpress branch September 24, 2021 17:02
@samiff samiff removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Sep 24, 2021
matticbot pushed a commit to Automattic/jetpack-changelogger that referenced this pull request Sep 24, 2021
@samiff @matticbot
Changelogger: try a new format (#21140)
bd10f30 
* Changelogger: try a new format

See #21138

* Add changelog

* Start new release cycle

* Update projects/packages/changelogger/src/Plugins/WordpressVersioning.php

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>

* Add more test cases

* Add new possible string for pre-releases

* Fix tested Atomic version number

* Updaet comparisons between versions

* Fix wrong test version

* Fix typo

* parsePrerelease to return an array of matched vals

See: Automattic/jetpack#21140 (comment)

* Fix typo

* [not verified] Apply feedback

Automattic/jetpack#21140 (review)

Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
Co-authored-by: Samiff <samiff@users.noreply.github.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1270677373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anomiex anomiex anomiex approved these changes

+1 more reviewer

@samiff samiff samiff left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jeherve jeherve

Labels

[Package] Changelogger [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jeherve @anomiex @samiff