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: add versioning reference #4277

Conversation

joshblack
Copy link
Contributor

@joshblack joshblack commented Oct 9, 2019

Add versioning reference for teams. This should explain what our policy is with respect to versioning and what types of changes in carbon-components-react correspond to which semver bumps

Changelog

New

  • Add versioning.md guide

Changed

Removed

@joshblack joshblack requested a review from a team as a code owner October 9, 2019 18:47
@ghost ghost requested review from abbeyhrt and asudoh October 9, 2019 18:47
@joshblack joshblack requested a review from vpicone October 9, 2019 18:47
@joshblack joshblack requested a review from emyarod October 9, 2019 18:49
@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for the-carbon-components ready!

Built with commit 3a34e18

https://deploy-preview-4277--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-components-react ready!

Built with commit 3a34e18

https://deploy-preview-4277--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-elements failed.

Built with commit 3a34e18

https://app.netlify.com/sites/carbon-elements/deploys/5d9e2b36bca54400087f0ee7

Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

Looks good! Definitely made minor/major and patch release changes more clear to me 🎉

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-elements failed.

Built with commit 799cfe1

https://app.netlify.com/sites/carbon-elements/deploys/5d9e2e87f7dfee00080d7cc6

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-components-react failed.

Built with commit 799cfe1

https://app.netlify.com/sites/carbon-components-react/deploys/5d9e2e870820c8000bbd0485

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-elements failed.

Built with commit 6af2590

https://app.netlify.com/sites/carbon-elements/deploys/5da12dbd4398e50008189b33

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for carbon-components-react ready!

Built with commit 6af2590

https://deploy-preview-4277--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for the-carbon-components failed.

Built with commit 23aadbd

https://app.netlify.com/sites/the-carbon-components/deploys/5d9e31059affd3000a2d97c9

@netlify
Copy link

netlify bot commented Oct 9, 2019

Deploy preview for the-carbon-components ready!

Built with commit 6af2590

https://deploy-preview-4277--the-carbon-components.netlify.com


function ExampleComponent() {
function onChange(arg) {
// If a or b change types (from number to string) or if they are removed
Copy link
Member

Choose a reason for hiding this comment

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

should there be a diff in this example? even just for the comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be down! Have any suggestions?

Copy link
Contributor

@vpicone vpicone left a comment

Choose a reason for hiding this comment

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

Should we add a note irt properties being added/taken away from arguments? Should adding a new property to an object be considered a breaking change?

@joshblack
Copy link
Contributor Author

@vpicone I'd be down! Feel free to commit any/all examples, would love more details

@vpicone
Copy link
Contributor

vpicone commented Oct 10, 2019

@joshblack Not sure if it's worth adding, what do you think? Something like 'we reserve the right to add new properties to callbacks' so don't do anything fishy like map over properties or something.

const onClick = () => {
  onClickProp({a: 'foo', b:'foo'})

  // property added ✅ minor release?
  onClickProp({a: 'foo', b:'foo', c: 'new property'}) 

  // property removed ❌ major release
  onClickProp({a: 'foo' }) 
}

Even though it's not a change in primitive type, it could be kind of a gray area. Not sure this matters when we normalize these event handlers across components.

@joshblack
Copy link
Contributor Author

@vpicone I say go for it, better to have more examples! Feel free to add onto this one, or I can merge and you can do another PR.

Only nit would be for styles that we use:

function onClick() {
  // ...
}

// Or even
function handleOnClick() {
  onClick({ a: 'string', b: 0 });
  // ...
}

Just for consistency in the file 👍

@joshblack joshblack merged commit ce42de3 into carbon-design-system:master Oct 12, 2019
@joshblack joshblack deleted the docs/add-breaking-change-prop-types branch October 12, 2019 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants