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

Jump to bottom

build-tools: Add release command and support for commands that use a state machine to govern logic #11706

Merged
merged 10 commits into from Sep 6, 2022
Merged

build-tools: Add release command and support for commands that use a state machine to govern logic #11706

merged 10 commits into from Sep 6, 2022

Conversation

tylerbutler
Copy link
Member

@tylerbutler tylerbutler commented Aug 26, 2022 •
edited

Follow up to #11705.

  • Adds an abstract base command class for commands that use state machines. This base command contains the scaffolding for commands that have internal state machines, including some logging for each state transition and handling the Init and Failed states.
  • Adds various interfaces and types for working with the state machine.
  • Adds the Fluid Unified Release Process finite state machine, which is used by the release command.

This PR also implements the release command, which is used to ensure that a release branch is in good condition, then walks the user through the release.

The release process and state machine

The release process itself is described by a state machine. This is a visual representation:

image

The basic flow of the command is:

  • The command starts and oclif (our CLI infrastructure) parses flags and arguments, validates them, etc.
  • The command's run method is called, which calls an internal stateLoop function.
  • That function loops through the machine states, passing each state to the command's state handler function.
  • The state handler handles the states it knows about, and if it doesn't handle the state, it passes it to its parent's state handler. Thus, the parent classes are relevant to the overall implementation of the state handling. In previous iterations there was a deeper class hierarchy, but now there's just one base abstract state machine command and the release command that inherits from it.

Reviewer guidance

I would particular like feedback on:

  • How the code is organized - both at the file level and at the module level.
  • It is a goal that given the name of a state and its code, it should be easy to understand what it's doing and the conditions under which it succeeds or fails. Did I succeed? The big picture is better understood using the state machine diagram.

Anything big will be punted to a follow-up PR so I can get this baseline in.

@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Aug 26, 2022
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 27, 2022
View reviewed changes
build-tools/packages/build-cli/src/lib/bump.ts Outdated

if (releaseGroupOrPackage instanceof MonoRepo) {
workingDir = releaseGroupOrPackage.repoPath;
cmd = `npx lerna version ${translatedVersion.version} --no-push --no-git-tag-version -y && npm run build:genver`;

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input

[String concatenation](1) based on [library input](2) is later used in [shell command](3).
build-tools/packages/build-cli/src/lib/bump.ts Outdated
cmd = `npx lerna version ${translatedVersion.version} --no-push --no-git-tag-version -y && npm run build:genver`;
} else {
workingDir = releaseGroupOrPackage.directory;
cmd = `npm version ${translatedVersion.version}`;

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input

[String concatenation](1) based on [library input](2) is later used in [shell command](3).
@github-actions github-actions bot added area: contributor experience public api change Changes to a public API and removed public api change Changes to a public API labels Aug 31, 2022
@tylerbutler tylerbutler marked this pull request as ready for review August 31, 2022 23:21
@tylerbutler tylerbutler requested review from msfluid-bot and a team as code owners August 31, 2022 23:21
@tylerbutler tylerbutler changed the title build-tools: Add support for commands that use a state machine to govern logic build-tools: Add release command and support for commands that use a state machine to govern logic Aug 31, 2022
@tylerbutler tylerbutler mentioned this pull request Sep 1, 2022
Merged
@tylerbutler tylerbutler mentioned this pull request Sep 2, 2022
Closed
Josmithr
Josmithr approved these changes Sep 6, 2022
View reviewed changes
build-tools/packages/build-cli/src/commands/release.ts

this.releaseGroup = flags.releaseGroup ?? flags.package!;
this.releaseVersion = context.getVersion(this.releaseGroup);
this.bumpType = flags.bumpType as VersionBumpType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do validation on these to ensure they are well formed?

build-tools/packages/build-cli/src/commands/release.ts Show resolved Hide resolved
build-tools/packages/build-cli/src/commands/release.ts Show resolved Hide resolved
build-tools/packages/build-cli/src/commands/release.ts Show resolved Hide resolved
build-tools/packages/build-cli/src/commands/release.ts Show resolved Hide resolved
build-tools/packages/build-cli/src/commands/release.ts
sections: [
{
title: "FIRST",
message: `Commit the local changes and create a PR targeting the ${context.originalBranchName} branch.\n\nAfter the PR is merged, then the release of ${this.releaseGroup} is complete!`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: looks like this string is duplicated below. Can we share a getter property or something?

build-tools/packages/build-cli/src/lib/branches.ts Show resolved Hide resolved
build-tools/packages/build-cli/src/machines/index.ts Outdated Show resolved Hide resolved
@tylerbutler @Josmithr
Update build-tools/packages/build-cli/src/machines/index.ts
c3d5424 
Co-authored-by: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com>
@tylerbutler tylerbutler merged commit 0de2738 into microsoft:main Sep 6, 2022
@tylerbutler tylerbutler deleted the cli/part-4 branch September 6, 2022 18:43
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

This was referenced Oct 26, 2022
Open
Open
@MarcelRaschke MarcelRaschke mentioned this pull request Nov 5, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonalivdeshpande sonalivdeshpande sonalivdeshpande approved these changes

@Josmithr Josmithr Josmithr approved these changes

@msfluid-bot msfluid-bot Awaiting requested review from msfluid-bot

Assignees
No one assigned
Labels
area: contributor experience base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tylerbutler @sonalivdeshpande @Josmithr