Standardize PVC component names #676
Comments
Part of #676 Renames: 1. `Primer::AutoComplete` to `Primer::Beta::AutoComplete` 2. `Primer::AutoComplete::Item` to `Primer::Beta::AutoComplete::Item` I had to update a bunch of configuration files since having a `stories/primer/beta/auto_complete` directory was making zeitwerk assume that `Primer::Beta::AutoComplete` was a module. The changes are: 1. Stop loading stories files when generating docs (prevents the zeitwerk issue) 2. Update `docs-build` CI to set an env var that will do the above 3. Creating `stories/primer/beta/auto_complete.rb` to explicity set `Primer::Beta::AutoComplete` as a class for the stories build
part of #676 Moves `Primer::AvatarComponent` to `Primer::Beta::Avatar`
|
I made an embarrassingly nasty script in #701 that we can hack on that I think should save us a bunch of time in this work. |
Related to #676 ### Summary Since we want to migrate all of our components into modules indicating their status, automating the migration task will save us a lot of time. This PR creates a `thor` runner that does everything needed to move a component in the main `app/components` directory into `app/components/STATUS`. ### How to use ```sh bundle exec thor component_status_migrator MyComponentName bundle exec thor component_status_migrator MyComponentName --status beta ``` ### What it does 1. Moves component files into a `/STATUS` subdirectory. This includes: i. Controller ii. Template iii. Test iv. Story 2. Adds the `STATUS` module to the component and removes the `Component` suffix. i. E.g.: `Primer::AvatarStackComponent` becomes `Primer::Beta::AvatarStack`. 3. Renames the test class from `PrimerComponentTest` to `PrimerStatusComponentTest` 4. Adds a `require "primer/status/component_name"` to the story. i. This is needed because of a limitation on the storybook gem. 5. Changes the `nav.yml` entry of the component to reflect the new docs path. 6. Updates all references to the component in the project to use the new name with module. 7. Runs rubocop to fix any linting issues. 8. Regenerates the docs. 9. Regenerates static files. ### How it works This is based on Thor [generators](https://github.com/rails/thor/wiki/Generators. When running the script, thor will call all the **public** methods in the order they are defined in the file. The big difference between using `thor` and a shell script is the helpers `thor` provides. See the [`Thor::Actions`](https://rubydoc.info/github/wycats/thor/master/Thor/Actions) documentation. It is also easier to read the script since it is all written in ruby and most of our contributors are Ruby developers.
|
This issue has been getting stale for a while, and having a mix of naming conventions in the codebase is confusing for us library engineers and the developers using it. I think it would be good to make a concerted effort to migrate the component names (possibly in one dedicated release?) and be done with it. Manuel and I have been working on changes to the migration script so that we can alias the old name to the new name, thereby running the migration in a non-breaking way. See #975 I hope we have the script complete and tested this week so we can start the migration next week if we're all happy. |
|
Yesterday @hectahertz and I finished #975 so we're technically prepared for running these migrations, but we started thinking about the consequences of doing all these renames and didn't feel confident that the status of each component actually corresponds to the status criteria. A lot of the components were upgraded to beta based on old standards that didn't have acceptance criteria for accessibility or API reviews, and some may already meet the criteria for the next level. We decided that it's important for us to feel confident about all the statuses before integrating them into the component names. Our plan is to implement the component status checklist as currently used in Primer React (see https://github.com/github/primer/issues/339#issuecomment-983925993) and audit each component to assign an accurate status to it. We don't plan on remediating any issues apart from minor issues that are preventing the component from progressing to the next level. This gives @hectahertz and I a chance to familiarise ourselves with all our components and will make it much easier to report on maturity progression going forward. I'll reference the tracking issues here when I write them. cc: @yaili @colebemis |
|
Since #1441 was merged, Primer::FlexComponent and Primer::FlexItemComponent should be removed. |
|
Since #1650 was merged, Primer::LocalTime and Primer::TimeAgoComponent can be removed from this issue. |
|
Why is Primer::LinkComponent migrated to Primer::Beta::LinkComponent? I thought that the Component suffix should be removed from all of the components. |
oops! that was a mistake in the title of the PR. the component is |
|
The Pull Request: #1729 was merged, so Primer::TimelineItemComponent::BadgeComponent can be marked as completed. |
lesliecdubs
changed the title
lesliecdubs
changed the title
|
Since #1730 was merged, Primer::Navigation::TabComponent's check box should be checked off. |
|
Since #1877 was merged, Primer::Truncate can be checked off. |
|
Thanks @ollie-iterators. @hrs can you confirm #676 (comment) has been completed and update the summary task list if so? |
|
I don't think Primer::LayoutComponent has been migrated yet. It doesn't look like it has a status of deprecated in https://github.com/primer/view_components/blob/main/app/components/primer/layout_component.rb |
|
Yup, that's all right! |
|
Also, I don't know if #1263 has been fixed. There was a PR that was related to it that was merged, but it is still open. |
|
Nope, but I'm working on that one right this minute. Ought to be done before too much longer. 😄 |
|
Since #1263 was closed, this Issue can be closed as completed. |
Overview
We don't have a standard for our component names, having three different variants:
Primer::ButtonComponent- with suffixPrimer::AutoComplete- no suffixPrimer::Alpha::ButtonMarketing- status as a moduleThis causes confusion and a not ideal developer experience, so we should have a single naming standard.
Proposal
Rename all components to have their status as a module and remove the
Componentsuffix.Examples:
Primer::ButtonComponent->Primer::Beta::ButtonPrimer::AutoComplete->Primer::Beta::AutoCompleteRemoving the suffix will make working with components less verbose. Using the status as a module will help with status promotion because we can keep the previous version for some time, giving users time to migrate.
How to migrate a component
The majority of the work is done through the
component_status_migratorthor command. This command will move files, update references, create a deprecated component in place of the original, fix navigation in storybook and the doc site, and more.Run a migration
As an example, the above referenced
Primer::ButtonComponentcan be migrated with the following command line:bundle exec thor component_status_migrator ButtonComponent --status betaNote that this command specifically calls out the target status for the component in
--status beta. If this option is left off, it will default toalphaand move the component into the correspondingalphafolder andAlpha::namespace.Be sure to set the correct status when migrating a component. Valid status entries include:
Testing a migration
Prior to creating a pull request and marking it as ready for review, be sure to run all tests, check storybook, and check the docs site to ensure all updates have been made:
script/testscript/devhttp://localhost:5000to check storybooklocalhost:5400to check the doc siteCommon errors
You may run into errors with storybook or the docs site, due to previously built site information. For example, storybook may still have the original
Primer::ButtonComponententry while also having the updatedPrimer::Beta::Buttonentry. If this happens, you will need to clean out the build cache and rebuild. One way to do that is to run the following:git clean -dfxscript/setupThe first call to clean will remove all untracked files and folders that are not part of
.gitignore. The second call will remove all untracked files and folders that are part of.gitignore. This will remove everything, including nod_modules and bundle vendor folders. After that, you'll need to runscript/setupto reinstall and rebuild before runningscript/devagain.Example pull requests
See the component migration work that is already completed, below, for example pull requests.
Work TODO
component status migrator code changes:
component_status_migrator.thorscript to handle--status stableUpdate component migration to support status of "stable" #1262component_status_migrator.thorto support migrating fromalphaorbetafolder, to correct status destination component status migrator can't migrate out of alpha or beta subfolders #1263todo / in progress component migrations:
Primer::Navigation::TabComponenttoPrimer::Alpha::Navigation::Tab#1730Primer::TimelineItemComponent::BadgeComponentin favor ofPrimer::Beta::TimelineItem::Badge#1729completed component migrations:
AvatarStackComponenttoBeta::AvatarStack#707ClipboardCopytoBeta::ClipboardCopy#1674Primer::LocalTimein favor ofPrimer::Beta::RelativeTime#1687Primer::MarkdowntoPrimer::Beta::Markdown#1681Primer::MenuComponenttoPrimer::Alpha::Menu#1682Primer::OcticonComponenttoPrimer::Beta::Octicon#1690Primer::OcticonSymbolComponenttoPrimer::Alpha::OcticonSymbols#1696Primer::SpinnerComponenttoPrimer::Beta::Spinner#1698Primer::StateComponenttoPrimer::Beta::Stateand deprecate the original #1716Primer::SubheadCompononenttoPrimer::Beta::Subhead#1720Primer::TabContainerComponenttoPrimer::Alpha::TabContainer#1725Primer::TimeAgoComponentin favor ofPrimer::RelativeTime#1692Primer::HellipButtontoPrimer::Alpha::HellipButton#1726The text was updated successfully, but these errors were encountered: