-
Notifications
You must be signed in to change notification settings - Fork 115
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
Conversation
|
cce3b74
to
2b947eb
Compare
a4ccf3e
to
663c8c4
Compare
There was a problem hiding this 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.
@siddharthkp gave me these tips:
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. |
@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?
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.
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. |
No 🤣 I had the same question. It's easy to change the source of the components though
Okay I created an issue here: gr2m/primer-changesets-cli#3 Looks like |
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.
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, |
Hey @gr2m this is looking good 👍 As soon as these merge conflicts are resolved we should be good to go. |
I added a CI check that not only checks for the presence of I realized that |
This should be ready to be merged |
Yeah, we should use |
See: #2089 |
The failing CI is not required, but the error seems related to changes in this pull request:
I'll have another look |
Nevermind, I'm pretty sure the |
@camertron I'm not authorized to merge, but this is good to go |
Description
This is part of an effort to provide a notification system to users of Primer releases.
To test the new
changeset
CLInpm install
npx changeset
Screenshots
Note that this recording shows the option for
@primer/react
. It will show these components when run inside theprimer/view_components
repositoryprimer-changeset-cli.mov
Integration
No
Merge checklist