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

replace @changesets/cli with primer-changesets-cli #2038

Merged
merged 15 commits into from Jun 29, 2023

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented May 31, 2023

Description

This is part of an effort to provide a notification system to users of Primer releases.

To test the new changeset CLI

  • Checkout this pull request
  • npm install
  • npx changeset

Screenshots

Note that this recording shows the option for @primer/react. It will show these components when run inside the primer/view_components repository

primer-changeset-cli.mov

Integration

Does this change require any updates to code in production?

No

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews

@gr2m gr2m requested review from a team and keithamus May 31, 2023 21:02
@changeset-bot
Copy link

changeset-bot bot commented May 31, 2023

⚠️ No Changeset found

Latest commit: 28ebbd0

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
Contributor

@camertron camertron left a comment

Choose a reason for hiding this comment

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

Hey @gr2m this looks great! I'm curious where the list of components comes from though. For example, BranchName and FilteredSearch aren't available in PVC. I would like to make sure PVC components that aren't available in PRC are represented in the list as well.

@gr2m
Copy link
Contributor Author

gr2m commented Jun 1, 2023

@siddharthkp gave me these tips:

  1. @primer/react: use the generated/components.json file
  2. @primer/view_components: retrieve components from https://primer.github.io/view_components/components.json

When you run the CLI it will log out where it retrieves the components.

Ideally I'd like it to retrieve the componenents in both cases from local files.

@camertron
Copy link
Contributor

@gr2m interesting, thanks for that explanation. I looked through our codebase and can't figure out what generates components.json, do you know where that happens?

When you run the CLI it will log out where it retrieves the components.

Ah ok I was a bit thrown off by the screen recording, but upon closer inspection it was recorded inside the primer/react repo as opposed to PVC.

Ideally I'd like it to retrieve the components in both cases from local files.

There are a number of other files in the static/ directory that contain all the information you're looking for. Check out static/info_arch.json.

@gr2m
Copy link
Contributor Author

gr2m commented Jun 1, 2023

I looked through our codebase and can't figure out what generates components.json, do you know where that happens?

No 🤣 I had the same question. It's easy to change the source of the components though

There are a number of other files in the static/ directory that contain all the information you're looking for. Check out static/info_arch.json.

Okay I created an issue here: gr2m/primer-changesets-cli#3

Looks like static/info_arch.json has 7 more components. Also it includes the capitalized module names, which I guess is preferable?

@camertron
Copy link
Contributor

No 🤣 I had the same question. It's easy to change the source of the components though

Ok cool 😎 If we can figure out what generates components.json I'd be happy to update it to include the missing components and/or include the fully-qualified constant names, but barring that info_arch.json should work.

Looks like static/info_arch.json has 7 more components. Also it includes the capitalized module names, which I guess is preferable?

Yes, definitely. Unlike React, our components go through a series of stages, i.e. experimental -> alpha -> beta -> stable, which is reflected in the component's namespace. In other words, Primer::Alpha::Label and Primer::Beta::Label are actually different components.

@camertron
Copy link
Contributor

Hey @gr2m this is looking good 👍 As soon as these merge conflicts are resolved we should be good to go.

@camertron camertron added the skip changeset Pull requests that don't change the library output label Jun 5, 2023
@gr2m gr2m marked this pull request as draft June 22, 2023 02:22
@gr2m gr2m marked this pull request as ready for review June 22, 2023 02:29
@gr2m
Copy link
Contributor Author

gr2m commented Jun 22, 2023

I added a CI check that not only checks for the presence of .changesets/*.md files, but also validates the <!-- Changed components: ... --> footer: https://github.com/gr2m/primer-release-changesets-validator-action

I realized that primer/react use skip changeset as label, while primer/view_components is using skip changelog. No problem, I'll make the CI work with both, but maybe it would make sense to normalize that across the repositories?

@gr2m
Copy link
Contributor Author

gr2m commented Jun 22, 2023

This should be ready to be merged

@camertron
Copy link
Contributor

camertron commented Jun 22, 2023

Yeah, we should use skip changeset everywhere. Do you happen to know what will happen to existing PRs if we change the name of a label?

@camertron
Copy link
Contributor

See: #2089

@gr2m
Copy link
Contributor Author

gr2m commented Jun 28, 2023

The failing CI is not required, but the error seems related to changes in this pull request:

invalid reference: primer-changesets-cli

I'll have another look

@gr2m
Copy link
Contributor Author

gr2m commented Jun 28, 2023

The failing CI is not required, but the error seems related to changes in this pull request:

invalid reference: primer-changesets-cli

I'll have another look

Nevermind, I'm pretty sure the primer-changesets-cli is the branch name, not related to the new npm package.

@gr2m
Copy link
Contributor Author

gr2m commented Jun 28, 2023

@camertron I'm not authorized to merge, but this is good to go

@camertron camertron merged commit f72e60f into primer:main Jun 29, 2023
@gr2m gr2m mentioned this pull request Oct 14, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changeset Pull requests that don't change the library output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants