-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
docs: add versioning reference #4277
Conversation
…black/carbon into docs/add-breaking-change-prop-types
Co-Authored-By: Connor <connor@ibm.com>
Deploy preview for the-carbon-components ready! Built with commit 3a34e18 https://deploy-preview-4277--the-carbon-components.netlify.com |
Deploy preview for carbon-components-react ready! Built with commit 3a34e18 https://deploy-preview-4277--carbon-components-react.netlify.com |
Co-Authored-By: Connor <connor@ibm.com>
Deploy preview for carbon-elements failed. Built with commit 3a34e18 https://app.netlify.com/sites/carbon-elements/deploys/5d9e2b36bca54400087f0ee7 |
There was a problem hiding this 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 🎉
Deploy preview for carbon-elements failed. Built with commit 799cfe1 https://app.netlify.com/sites/carbon-elements/deploys/5d9e2e87f7dfee00080d7cc6 |
Deploy preview for carbon-components-react failed. Built with commit 799cfe1 https://app.netlify.com/sites/carbon-components-react/deploys/5d9e2e870820c8000bbd0485 |
Deploy preview for carbon-elements failed. Built with commit 6af2590 https://app.netlify.com/sites/carbon-elements/deploys/5da12dbd4398e50008189b33 |
Deploy preview for carbon-components-react ready! Built with commit 6af2590 https://deploy-preview-4277--carbon-components-react.netlify.com |
Deploy preview for the-carbon-components failed. Built with commit 23aadbd https://app.netlify.com/sites/the-carbon-components/deploys/5d9e31059affd3000a2d97c9 |
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this 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?
@vpicone I'd be down! Feel free to commit any/all examples, would love more details |
@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. |
@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 👍 |
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 bumpsChangelog
New
Changed
Removed