Skip to content

[HOLD]chore: update changeset-snapshot-publish script to accept a parameter… #5366

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

blunteshwar
Copy link
Contributor

@blunteshwar blunteshwar commented Apr 11, 2025

… for versioning

Description

Enhanced Changeset Publishing for Snapshots and Beta Releases
Changes
Updated changeset-snapshot-publish script to accept a parameter for snapshot identification
Enhanced beta release workflow to dynamically determine the next beta version
Updated version management in the release pipeline
Purpose
These changes improve our release automation by:
Allowing custom snapshot identifiers when publishing test/development versions
Automating the beta versioning process to consistently increment beta numbers
Centralizing version management in package.json without requiring external scripts
Implementation
Added parameter support to the snapshot publish command using shell parameter substitution
Implemented dynamic beta number determination based on previously published versions
Maintained backward compatibility with existing workflows
This update primarily affects snapshot releases used for testing purposes, providing a more flexible and maintainable approach to our release pipeline.

Related issue(s)

Motivation and context

How has this been tested?

  • Test case 1

    1. Go here
    2. Do this
  • Test case 2

    1. Go here
    2. Do this
  • Did it pass in Desktop?

  • Did it pass in Mobile?

  • Did it pass in iPad?

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

@blunteshwar blunteshwar requested a review from a team as a code owner April 11, 2025 15:02
Copy link

changeset-bot bot commented Apr 11, 2025

⚠️ No Changeset found

Latest commit: abcadcd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

Copy link

Tachometer results

Currently, no packages are changed by this PR...

@@ -22,7 +22,7 @@
"build:ts:watch": "wireit",
"build:types": "wireit",
"build:watch": "wireit",
"changeset-snapshot-publish": "yarn prepublishOnly && yarn changeset version --snapshot && yarn lint:versions --fix && yarn update-version && yarn changeset publish --no-git-tag --tag snapshot",
"changeset-snapshot-publish": "yarn prepublishOnly && yarn changeset version --snapshot ${1} && yarn lint:versions --fix && yarn update-version && yarn changeset publish --no-git-tag --tag snapshot",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as a heads up, this kind of positional parameter does not translate well across operating systems. The most cross-system way to pass variables to commands is via a node script using yargs to read in from the command line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I appreciate your feedback about cross-platform parameter handling. You raise a valid point about positional parameters potentially causing issues across different operating systems.
While I agree that a Node script with yargs would be the most robust cross-platform solution, I believe this approach should work well for our current needs because:

  • Modern shells on macOS, Linux, and Windows (with appropriate tools) do support this type of parameter substitution
  • The Yarn/npm CLI handles these parameters consistently across platforms
  • This is specifically for snapshot releases, which we'll primarily be using for testing purposes
  • The implementation keeps all our command logic centralized in package.json
    The approach is straightforward and avoids the overhead of creating and maintaining separate script files
    Since this is just for snapshot/test releases rather than production deployments, the simplicity of this approach seems appropriate for our needs. If we encounter any compatibility issues, I'll certainly reconsider implementing a proper Node script solution. Thank you for the helpful suggestion!

Copy link
Collaborator

Choose a reason for hiding this comment

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

That all makes great sense! Thanks for outlining some of the background you did to validate this. This is only meant to be run in CI, is that correct? If we have anyone on a unique set-up, can we ask them to validate this locally if there's a chance it might need to be run manually?

@caseyisonit
Copy link
Contributor

@blunteshwar does this conflict with rubens PR? as in does his PR need to go in first or does this PR? Trying to understand merge order

@blunteshwar
Copy link
Contributor Author

@blunteshwar does this conflict with rubens PR? as in does his PR need to go in first or does this PR? Trying to understand merge order

This is an alternative to ruben's pull request he's trying to achieve the same using snapshot releases, which is meant for testing purpose release and will not generate exact beta versions which we were generating earlier.
This Pr leverages the beta release number to generate the exact beta versions without changing existing release functionality

@blunteshwar
Copy link
Contributor Author

.

@blunteshwar blunteshwar reopened this Apr 11, 2025
@blunteshwar blunteshwar reopened this Apr 11, 2025
@blunteshwar blunteshwar changed the title chore: update changeset-snapshot-publish script to accept a parameter… [HOLD]chore: update changeset-snapshot-publish script to accept a parameter… Apr 11, 2025
@blunteshwar blunteshwar self-assigned this Apr 11, 2025
- name: Get current package version and latest beta number
id: version-info
run: |
# Fetch the latest beta version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @blunteshwar, I love that you're interested in tackling this workflow for us. I think it'll be a big improvement on how we can communicate some of these not-yet-ready for production releases to our consumers.

I wanted to mention a few things here that could help frame your work.

  1. Changesets has workflows for both pre-releases (alphas, betas, release candidates) as well as snapshots (timestamped one-off releases, "canary" or "nightly" releases). It looks like you might be combining both of those concepts here, but perhaps you intended to only focus on "snapshot" releases.
  2. Based on the docs links in the first point, depending on which type you choose, you might not need to perform the steps here to reason about the current and next version numbers. Especially if you decide that you want to use the snapshot workflow because it's all based on a timestamp which will always be incrementally created.
  3. If you want a pre-release instead of a snapshot, I'm pretty sure we can still lean on Changesets to handle the incremental numbering, but we can have a conversation about that if necessary. It's slightly more involved, but the preferred "Changesets way" of doing this involves branching.
  4. Finally, it is possible to get a little creative with GitHub Actions and Changesets by doing something similar to what we've done in Spectrum CSS or what Garth has done in Spectrum Tokens. This approach allows authorized repo contributors to publish snapshots via the GitHub Actions "run workflow" UI, without any CI or CLI needed.

Lots of possibilities with this tool! Please reach out to me, Cassondra, or Garth if you'd like to talk through this a bit more.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is what it looks like in the "run workflow UI"
Screenshot 2024-09-04 at 4 06 00 PM

And this is what it looks like on npm after that workflow completes
spectrum-css_page-_npm

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.

4 participants