docs: add versioning guide

[joshblack]

Add a versioning guide to contributor-docs/versioning.md to see what kind of consensus the team has for types of changes and what semver bumps they correspond to 👀

Would be awesome if this could be a living document that we can refer to whenever different types of scenarios come up in order to consistently apply our understanding of semver against @primer/react.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 37cfd07

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

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	89.03 KB (0%)
dist/browser.umd.js	89.64 KB (0%)

[broccolinisoup]

Thank you so much for writing this up! I left a few comments, looking forward to hearing your thoughts 🙌🏼

[broccolinisoup]

For a full list of changes that the team
commits to being backwards-compatible, view the changes table
below

Is this sentence meant to be for a table that only lists minor and patch?
What would you think grouping them by Category firstly (as you already done) and the semver bump secondly?

It might be more digestible and easier to review maybe?

Colouring them accordingly would be super cool too but I am mindful of introducing HTML into MD.

Let me know your thoughts 🙌🏼

[joshblack]

@broccolinisoup you're totally right, the wording is off. It should just be types of changes. Updating now!

For grouping, happy to try out either that makes it easier to reference. Right now the intent is to group by area (component, prop, prop type, etc) and list out the changes that one can expect to that specific area within a category.

Let me know if that makes it harder to look up stuff though, it may be that listing by bump type is easier. If bump type is used, how would you prefer sorting the type of change column?

[broccolinisoup]

Sorry, I didn't provide an example so it made it blur. I love the way you group by the area and list the out the changes to expect. The only difference I was thinking, instead of randomly listing the changes under a specific area, we could list the patch changes first, minor second and major as the last.

Instead of

| Props      | A prop is added to a component                                | `minor`     |
|            | The type of a prop is made more general                       | `minor`     |
|            | The type of a prop is made more specific                      | `major`     |
|            | A prop is deprecated                                          | `minor`     |
|            | A prop is removed from a component                            | `major`     |

We could maybe do

| Props      | A prop is added to a component                                | `minor`     |
|            | The type of a prop is made more general                       | `minor`     |
|            | A prop is deprecated                                          | `minor`     |
|            | The type of a prop is made more specific                      | `major`     |
|            | A prop is removed from a component                            | `major`     |

Let me know what you think 🙌🏼

[broccolinisoup]

Should we add a section for Accessibility or things related to accessibility can go under other corresponding sections?

[joshblack]

@broccolinisoup makes sense to me! What kind of situations would be good to place under an Accessibility category?

[broccolinisoup]

I guess things in my mind can certainly go under other categories but maybe explicitly having a section for Accessibility feels more inclusive? 🙂

I was thinking;

• Exposing a mandatory aria attribute in the API
• Introducing a11y remediations that result in breaking changes in a current user behaviour
• Updating keyboard interactions

Does any of it sound applicable at all?

[rezrah]

- ... a `minor` or `patch` update for a package from the Primer you should feel confident ...
+ ... a `minor` or `patch` update to a package from Primer you should feel confident ...

[rezrah]

This one's interesting:

1. In Primer Primitives deprecating something doesn't constitute a material breaking change in the output, but serves as a flag that something will soon be removed and that we have provided an alternative to ease migration
2. During v35 major release, I recall the team suggested in a retro that we don't make deprecations major. We had the same feedback from library users.

[joshblack]

@rezrah I definitely agree, deprecations in minor releases feel more appropriate/align with expectations.

[broccolinisoup]

During v35 major release, I recall the team suggested in a retro that we don't make deprecations major. We had the same feedback from library users.

I am very curious to learn more about this. It is very interesting.

We will be deprecating UnderlineNav in v36 and it is planned to be a major change. We re-wrote the component entirely and the public API has changed. I don't have much experience with versioning but I am super keen to improve! I am curious to know if it is still possible to refactor UnderlineNav and introduce as a minor change with warning the deprecation and providing the new API as an alternative 🤔 Would it worth giving a try or not really at this stage?

Current UnderlineNav
Newly written UnderlineNav

[rezrah]

@joshblack @broccolinisoup I've somehow found the FigJam file I was referencing in my earlier comment. You should receive a link to it in your emails.

@broccolinisoup - my reading of your situation based on the diagram would be to issue the deprecation notice through comms (docs, changelogs, etc) and a patch (or minor) update of the libraries (for JSDOC @deprecate annotations). Main thing is to leave it in-place. We're just issuing notice AoT.

Then wait for the next major release (~couple of weeks) to move it into the deprecated bundle and move UnderlineNavV2 from drafts into main bundle. Eventually few months after, we can remove deprecated component altogether.

Could this still work for you?

[broccolinisoup]

I've somehow found the FigJam file I was referencing in my earlier comment. You should receive a link to it in your emails.

Thanks, this is brilliant 🙌🏼

Could this still work for you?

@rezrah Thank you so much for clarifying the process! According to your explanation, we are on track. The only thing is different than the plan is updating the old UnderlineNav with the @deprecate status as a patch (or minor) update before the major release. Would that still be a good practise without promoting the new UnderlineNav as an alternative component (As this plan to come as a major later)? I think this is what I am trying to wrap my head around.

Thank you so much 🙏🏼

[broccolinisoup]

Hi! @rezrah just wanted to resurface this, would love to hear your thoughts 😊

[rezrah]

Hey @broccolinisoup, sorry I missed this earlier 😞.

Would that still be a good practise without promoting the new UnderlineNav as an alternative component

I think so, because the idea behind issuing a deprecation notice before creating a breaking change (as a result of moving from main bundle to deprecated) is that we're giving notice of a deprecation ahead of time so there are no surprises. Some of the recurring feedback received after the v35 release from teams around GitHub was that replacing certain deprecated components was not on their priority list, but upgrading to a major is still important. Had they known ahead of time that we were deprecating a component, they could have been better prepared for it.

I think this problem is exacerbated by us having a separate bundle for deprecated components.

TL;DR the proposal in my suggestion is:

1. Issue @deprecated notice for UnderlineNav in next patch/minor release. At same time, provide the new UnderlineNav in the drafts bundle and update any relevant docs about this upcoming change.
2. Release a technical preview. See here for v35 example.
3. Wait for next major release.
4. Cut the major release and Move UnderlineNav(v1) into deprecated and UnderlineNav(v2) into main bundle.

What do you think?

[broccolinisoup]

That sounds great! I appreciate the very detailed explanation as well 🙇🏻‍♀️ I'll capture this info in the deprecation process documentation PR.

[rezrah]

Do you have any examples of this one in mind? I think this could also be a patch in certain scenarios?

[joshblack]

Really just was thinking of a narrow case where a type has been added to the Public API of the package.

In this scenario, I framed it as a minor since it would be additive and would require someone to explicitly import it in order to use it.

Let me know if there is a better way to talk about types in this kind of context, I am honestly not sure how to frame type changes in terms of semver so was just applying a propTypes mindset to this kind of section 👀

[rezrah]

ah gotcha, thanks for clarifying. I think a type addition that doesn't make a material API change, or something that doesn't require the user to react to can go into a patch. A prominent new feature however, like a new component entirely - which includes new types - would constitute a minor in my mind. I agree that it's nuanced however. Perhaps we can split this up into different type edge cases.

[github-actions]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

[github-actions]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.