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

Tools: Add steps to release script #40556

Merged
merged 14 commits into from
Dec 18, 2024
Merged

Conversation

tbradsha
Copy link
Contributor

This is a very light refactor of the release script to allow one to resume from a specific release step. This allows one to abort the script to adjust something without having to start over or do everything manually.

The main changes are as follows:

  • Option parsing was rewritten
  • Plugin/version parsing was rewritten, and it now accepts projects/plugins/X, plugins/X, and X
  • A -s flag was added to allow one to specify a step
  • Code blocks were moved into functions
  • CUR_STEP is used in conjunction with RELEASE_STEPS to determine which steps to run
  • verify_prerelease_branch was added to relevant steps to prevent execution if not on the correct branch
  • The step number and associated function is output at the beginning of each step
  • A currently-unsed generate_resume_command function was added in case at some point we decide to hook it into a trap or something

I also added support for a JP_DISABLE_TRACKS flag in send_tracks_event so we can prevent tracks from being sent during testing.

Changes to the processes within the steps is out of scope for this PR and can be considered for a future PR. For now, this should be considered an advanced feature for those that are willing to take responsibility for what happens (there's no step rollback feature). :^)

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

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

Testing instructions:

  • Try this out with an actual release?
  • Try aborting during step 1 (e.g. while editing the changelog) and then resuming with the -s flag
  • Jump to the last step with something like this: ./tools/release-plugin.sh -s 8 crm boost jetpack

Copy link
Contributor

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.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  • ✅ 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.


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Choose a review path based on your changes:
    • A. Team Review: add the "[Status] Needs Team Review" label
      • For most changes, including minor cross-team impacts.
      • Example: Updating a team-specific component or a small change to a shared library.
    • B. Crew Review: add the "[Status] Needs Review" label
      • For significant changes to core functionality.
      • Example: Major updates to a shared library or complex features.
    • C. Both: Start with Team, then request Crew
      • For complex changes or when you need extra confidence.
      • Example: Refactor affecting multiple systems.
  3. Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

tools/includes/send_tracks_event.sh Outdated Show resolved Hide resolved

Conduct a full release of specified plugins through release branch creation. Just the plugin name is fine, such as 'jetpack' or 'backup.' The version is optional, and if not specified, will be set to the next stable version.
Usage:
Copy link
Contributor

Choose a reason for hiding this comment

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

Our other scripts, as well as examples like git --help and jq --help, all lead with usage and have the long description underneath.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair; updated in 40c39d9.


Conduct a full release of specified plugins through release branch creation. Just the plugin name is fine, such as 'jetpack' or 'backup.' The version is optional, and if not specified, will be set to the next stable version.
Usage:
$0 -h
Copy link
Contributor

Choose a reason for hiding this comment

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

Little need IMO to call out -h specially.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


Options:
-h Show this help message.
-h, --help Show this help message.
-s <step>, --step <step> Start at a given step.
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point we should probably give some way for a user to list the steps. A --list-steps option would work IMO.

But that can wait until it's not experimental anymore 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an undocumented --list-steps option: ab31162

The harder part in a future iteration is to give the steps more understandable names. :^)

tools/release-plugin.sh Outdated Show resolved Hide resolved
Comment on lines 403 to 413
# No args, help flag, or invalid flag, so show usage.
if [[ $# -eq 0 || $1 == '-h' || $1 == '--help' ]]; then
usage
elif [[ $1 == '-s' || $1 == '--step' ]]; then
if [[ $2 =~ ^[0-9]$ ]]; then
CUR_STEP="$2"
shift 2
else
PROJECTS["$SLUG"]="${PROJECTS[$PLUGIN]}"
unset "PROJECTS[$PLUGIN]"
usage
fi
done
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to switch away from getopts so we can have long options, it might be better to look to something like this as an example

while [[ $# -gt 0 ]]; do
arg="$1"
shift
case $arg in
-h|--help)
usage
exit
;;
--dev)
WHAT=dev
;;
--release)
WHAT=rel
;;
-*)
die "Unrecognized option \"$arg\""
;;
*)
PLUGINS+=( "$arg" )
;;
esac
done

The advantage there is that if someone adds --help anywhere in the command line it still gets processed correctly, instead of having to be the only option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

instead of having to be the only option.

More accurately: instead of having to be the first option.

Ideally BSD getopt would support long options. Gotta love the usage info on macOS:

$ getopt -h 
 --

The example you've listed gets a bit more complicated due to the version being optional, as the args could be jetpack --help, which would try to detect a version. Obviously a workaround would be to check if $1 or $2 was --help, which isn't pretty but is doable.

But then you have jetpack -s 8 vs. jetpack 19.2 -s 8. If $2 is -s then $3 is the step number, and if $1 is -s then $2 is the step number...it gets rather ugly fast.

The version currently in trunk doesn't handle it unless the first option either; getopts fails as soon as a positional param is found, so the behaviour is the same as always.

At this stage I'd rather just keep positional args after flags unless you feel strongly about it.

tbradsha and others added 6 commits December 12, 2024 12:57
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@tbradsha tbradsha requested a review from anomiex December 16, 2024 23:28
anomiex
anomiex previously approved these changes Dec 18, 2024
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 enough to merge and iterate on IMO. I didn't try to test it beyond --help and --list-steps though.

tools/release-plugin.sh Outdated Show resolved Hide resolved
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@tbradsha tbradsha merged commit 6382dad into trunk Dec 18, 2024
56 checks passed
@tbradsha tbradsha deleted the update/plugin_release_script branch December 18, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants