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

docs(adr): Increase ReactUSWDS version when updating USWDS version #1045

Merged
merged 6 commits into from
Apr 16, 2021

Conversation

suzubara
Copy link
Contributor

Summary

Adds an ADR to help us decide ReactUSWDS versioning as it relates to USWDS versions.

I also copied the ADR template @brandonlenz introduced in #1026

Related Issues or PRs

Resolves #343

@suzubara suzubara added type: documentation Improvements or additions to documentation help wanted Looking for assistance working on this labels Mar 23, 2021
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook March 23, 2021 23:17 Inactive
@suzubara suzubara mentioned this pull request Mar 24, 2021
## Considered Options

- Bump the ReactUSWDS major version when updating USWDS minor & major versions
- Bump the ReactUSWDS version with the same release type as USWDS
Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for Bump the ReactUSWDS version with the same release type as USWDS. We probably should also acknowledge this means we don't anticipate maintaining multiple active versions of the library. For example, we wouldn't back port a new component or bug fix to a previous USWDS version.

Comment on lines +40 to +41
- Bump the ReactUSWDS major version when updating USWDS minor & major versions
- Bump the ReactUSWDS version with the same release type as USWDS
Copy link
Contributor

@brandonlenz brandonlenz Mar 25, 2021

Choose a reason for hiding this comment

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

I'm comfortable with option 1 or option 2.

We know that uswds does release presentational changes that are sometimes not mentioned in the release notes, which we could consider breaking changes.

Reading the uswds release process makes me actually lean towards option 2. If our library is simply a react-wrapper for uswds, is there really any value to us being more strict than usdws? Option 1 would mean that projects depending on both libraries would have minor version updates for uswds and a major version updates for react-uswds for the same changes. Specifying the package.json dependencies on a consumer project would be weird in that context. For example, if you use ^2.10.0 for uswds, you wouldn't have a good equivalent for react-uswds without also opening yourself up to other breaking changes. It kind of sounds like we're wanting consumers to manually manage the versions of USWDS and ReactUSWDS, so maybe that concern isn't as big of a deal as I'm making it sound like.

Related note (to option 2):

I would've thought based on

the following should trigger a major version increment:

  • Changing the way in which elements with .usa- class names are structured in HTML

that the 2.10.1 change (a patch version change!)

Now we're using a p instead of an h3 for usa-footer__logo-heading

would have constituted a major version change.

But ultimately for me its coming back to "What does being more strict than the library we're wrapping ultimately buy us?". If consumers are expected to manually manage these versions, the stability of this library's versions isn't as big of a concern.

@haworku haworku removed the help wanted Looking for assistance working on this label Apr 1, 2021
@haworku
Copy link
Contributor

haworku commented Apr 1, 2021

Based on today's meeting I think we have consensus around option 2 🎉. Thank you @suzubara for gathering these options.

Next steps for implementing this ADR could be:

  • Document the decision and publish
  • Add section on USWDS version change to our docs. Maybe something like "Guidelines for changing USWDS version in library".
  • We also said in our meeting we expect USWDS version bump to happen with a separate PR/ticket rather than being combined into the work for a specific ticket. We should probably briefly document this point as well. Maybe we can a uwsds version label for those PRs.

@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 6, 2021 14:42 Inactive
@suzubara suzubara force-pushed the sr-uswds-version-adr branch from ef7720c to 1129029 Compare April 12, 2021 17:34
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 12, 2021 17:52 Inactive
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 12, 2021 18:29 Inactive
@suzubara suzubara requested review from brandonlenz and ahobson April 12, 2021 18:41
@suzubara suzubara marked this pull request as ready for review April 12, 2021 18:41
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 12, 2021 18:42 Inactive
@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 15, 2021 18:39 Inactive
Copy link
Contributor

@haworku haworku left a comment

Choose a reason for hiding this comment

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

Our first ADR 😭 😍 . Look at us deciding things.

@haworku
Copy link
Contributor

haworku commented Apr 15, 2021

Suggestion: also add this line to the first page of our README so its front and center.

It is suggested applications use the same version of USWDS that was used to build the version of ReactUSWDS they're using. A version mismatch may result in unexpected markup & CSS combinations.

@trussworks-infra-zz trussworks-infra-zz temporarily deployed to storybook April 16, 2021 15:53 Inactive
@suzubara suzubara merged commit 59f6720 into main Apr 16, 2021
@suzubara suzubara deleted the sr-uswds-version-adr branch April 16, 2021 16:06
brandonlenz pushed a commit that referenced this pull request May 12, 2021
…1045)

* Add draft of ADR

* Update ADR with chosen decision

* Added note about order of implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Add note on USWDS versions
5 participants