Skip to content

docs: add versioning guide #2753

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.

Sign up for GitHub

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

Closed
joshblack wants to merge 10 commits into from
Closed

docs: add versioning guide #2753

Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c2eb7ae
docs: add versioning guide
joshblack Jan 9, 2023
4d71ac8
docs: run doctoc
joshblack Jan 9, 2023
0a052f8
Update versioning.md
joshblack Jan 9, 2023
0af1868
docs(versioning): update about section and add todo notes for bumps
joshblack Jan 10, 2023
ca3559b
Update versioning.md
joshblack Jan 12, 2023
621640b
Update versioning.md
joshblack Jan 24, 2023
2b7c947
Merge branch 'main' into docs/add-versioning-docs
joshblack Jan 25, 2023
fa353fd
Add token reference
joshblack Jan 30, 2023
0800394
Update versioning.md
joshblack Jan 30, 2023
37cfd07
Update versioning.md
joshblack Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions contributor-docs/versioning.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# Versioning

<!-- prettier-ignore-start -->
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
## Table of Contents

- [Overview](#overview)
- [Changes](#changes)
- [Examples](#examples)
- [A prop is added to a component](#a-prop-is-added-to-a-component)
- [An existing prop is deprecated](#an-existing-prop-is-deprecated)
- [The DOM node that an `id` corresponds to is changed](#the-dom-node-that-an-id-corresponds-to-is-changed)
- [The DOM node that an `aria-label` corresponds to is changed](#the-dom-node-that-an-aria-label-corresponds-to-is-changed)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- prettier-ignore-end -->

## Overview

The Primer team aims to follow
[Semantic Versioning](https://semver.org/) (semver) for each of the packages
that we ship. From semver.org, this means that:

> Given a version number MAJOR.MINOR.PATCH, increment the:
>
> 1. **MAJOR** version when you make incompatible API changes,
> 2. **MINOR** version when you add functionality in a backwards compatible
> manner, and
> 3. **PATCH** version when you make backwards compatible bug fixes.
>
> _Additional labels for pre-release and build metadata are available as
> extensions to the MAJOR.MINOR.PATCH format._

As a result, whenever you see a `minor` or `patch` update for a package from the
Copy link
Contributor

@rezrah rezrah Jan 10, 2023

Choose a reason for hiding this comment

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

- ... 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 ...

Primer you should feel confident that you can update without
anything breaking in your project. For a full list of changes that the team
commits to being backwards-compatible, view the [changes](#changes) table
below.
Copy link
Member

@broccolinisoup broccolinisoup Jan 10, 2023

Choose a reason for hiding this comment

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

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 🙌🏼

Copy link
Member Author

@joshblack joshblack Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@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?

Copy link
Member

@broccolinisoup broccolinisoup Jan 13, 2023

Choose a reason for hiding this comment

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

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 🙌🏼


## Changes

| Category | Type of change | semver bump |
| :--------- | :------------------------------------------------------------ | :---------- |
| Component | A component is added | `minor` |
| | A component is deprecated | `minor` |
| | A component is removed | `major` |
| 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` |
| | The element to which additional props are added to is changed | |
| Markup | The DOM node that an `id` corresponds to is changed | |
| | The DOM node that an `aria-label` corresponds to is changed | |
| | The `role` of a component is changed | |
| Styles | The `position` of the outermost element is updated | |
| | The `display` property of the outermost element is updated | |
| | A flex or grid property is updated | |
| | A selector is added | |
| | A selector is removed | |
| | The specificity of a selector is raised | |
| | The specificity of a selector is lowered | |
| Behavior | Interactions are added to a component | |
| | Interactions are removed from a component | |
| TypeScript | A type is added | `minor` |
Copy link
Contributor

@rezrah rezrah Jan 10, 2023

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@joshblack joshblack Jan 10, 2023

Choose a reason for hiding this comment

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

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 👀

Copy link
Contributor

@rezrah rezrah Jan 12, 2023

Choose a reason for hiding this comment

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

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.

| | A type is made more general | `minor` |
| | A type is made more specific | `major` |
| | A type is removed | `major` |
| Package | An entrypoint is added to the package | `minor` |
| | An entrypoint is removed from a package | `major` |

Copy link
Member

@broccolinisoup broccolinisoup Jan 10, 2023

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@joshblack joshblack Jan 10, 2023

Choose a reason for hiding this comment

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

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

Copy link
Member

@broccolinisoup broccolinisoup Jan 13, 2023

Choose a reason for hiding this comment

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

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?

### Examples

#### A prop is added to a component

semver bump: **minor**

```diff
type Props = {
propA: string;
+ propB: string;
};

function ExampleComponent({
propA,
+ propB,
}: Props) {
return (
<>
<span>{propA}</span>
+ <span>{propB}</span>
</>
);
}
```

#### An existing prop is deprecated

semver bump: **minor**

```diff
type Props = {
propA: string;
+ /**
+ * @deprecated This prop will be removed in the next major version of
+ * `@primer/react`. Please use <replacement> instead.
+ */
propB: string;
};

function ExampleComponent({
propA,
+ propB,
}: Props) {
return (
<>
<span>{propA}</span>
+ <span>{propB}</span>
</>
);
}
```

#### The DOM node that an `id` corresponds to is changed

semver bump: **major**

#### The DOM node that an `aria-label` corresponds to is changed

semver bump: **minor**