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

[FEAT] Same version for NPM and the repo release #56

Closed
lucaburgio opened this issue Jun 26, 2021 · 13 comments
Closed

[FEAT] Same version for NPM and the repo release #56

lucaburgio opened this issue Jun 26, 2021 · 13 comments
Labels
enhancement New feature or request

Comments

@lucaburgio
Copy link
Collaborator

@paescuj @PianoManDanDan What about having the NPM version aligned with the version here on GitHub? See for ref feathericons/feather

@lucaburgio lucaburgio added the enhancement New feature or request label Jun 26, 2021
@paescuj
Copy link
Collaborator

paescuj commented Jun 28, 2021

Would make sense IMO. That would mean that we consider it as a monorepo, though. If something changes in "iconoir-react" for example and you would like to publish this, you would also need to bump the main "iconoir" version.

@PianoManDanDan
Copy link
Contributor

I did consider doing this initially, but decided against it to try and keep the npm package separate from the main iconoir library. As @paescuj said, if changes want to be made to any of the npm packages, you'll have to bump all the packages and the iconoir release too. Plus I thought if we start at v1.0.0 we can always go up to match the iconoir version later on, where we couldn't go down if we decided it was a bad move

@lucaburgio
Copy link
Collaborator Author

Makes sense guys, agree. Thank you for the feedback. Let's get rid of this for now then 👍

@paescuj
Copy link
Collaborator

paescuj commented Aug 8, 2022

I'd like to resume the discussion on this topic as I still feel like we should use separate versions for each package.
For example, I'd like update to the latest version of svgr in the near feature and release it under a new version. In fact, this would only affect the iconoir-react & iconoir-react-native packages, not the iconoir (core) package.
With the current solution and the case described above, users of the iconoir (core) package would receive an update too, although the update wouldn't affect them - same applies vice versa.
As the user base is growing, I want to make sure they can rely on the "semver" of each package.
IMO, the current solution doesn't scale well! We should deliver reliable versions adhering to the "semver" standard (especially since we're on NPM).

I think we could easily switch back to separate versions which would look something like this:

  • Breaking changes to the icons (for example, renaming an icon):
    • ✅ New major release of the iconoir (core) package
    • ✅ New major release of the sub packages (like iconoir-react)
  • New icons added to iconoir (means a "new feature" in terms of "semver"):
    • ✅ New minor release of the iconoir (core) package
    • ✅ New minor release of the sub packages (like iconoir-react)
  • Changes only affecting the iconoir package (for example, something like fix: css styles should now support color and font-size #141):
    • ✅ New release for the iconoir (core) package
    • 🚫 No new release for the sub packages (like iconoir-react)
  • Changes only affecting one of the sub package (for example, the case described above):
    • 🚫 No new release for the iconoir (core) package
    • ✅ New release for the corresponding sub package(s) (like iconoir-react)

To underline this change we could also consider switching to scoped packages, like @iconoir/core, @iconoir/react, but this might be worth a separate discussion...

I greatly appreciate your feedback @lucaburgio @sammarks!

@paescuj paescuj reopened this Aug 8, 2022
@sammarks
Copy link
Collaborator

My main thought is it's more consistent to have all of the packages on the same version rather than separate versions. This gets more important when we get the new website involved, which lists the current version in the top-left corner pretty prominently. It would be confusing to have that version say one thing when the packages say another depending on major / breaking changes or not.

This does mean we don't follow semver religiously, but I feel the consistency having all of the versions the same is a decent tradeoff.

@lucaburgio I do believe we should align our releases more with semver though. So for example, rather than the new website being a "6.0" release (which would be a major change, and indicate a breaking change in the packages), we would just make it a "5.X" release (which would be a minor change, and indicate new features but nothing breaking).

And pretty much any new icons from here on would be "5.X" releases, until we introduce a breaking change in any of the libraries, at which point it would be "6.X"

I could see an argument for using separate versions when a breaking change is introduced, because package managers won't automatically upgrade React users from 5.x to 6.x (in the event of a breaking change with the Flutter package, for example). But I suppose we could cross that bridge when we get there.

I also believe, with the way the packages are generated, if we introduced a breaking change in one of them we would likely be introducing a breaking change in all of them, so the discussion mentioned above might be moot.

Point being, my current opinion is with the goal of the project being mass adoption, reducing friction and the possibility for confusion should be the goal, and having consistent versions reduces complexity.

@paescuj
Copy link
Collaborator

paescuj commented Aug 23, 2022

Thanks @sammarks for your response!

My main thought is it's more consistent to have all of the packages on the same version rather than separate versions. This gets more important when we get the new website involved, which lists the current version in the top-left corner pretty prominently. It would be confusing to have that version say one thing when the packages say another depending on major / breaking changes or not.

I understand that and I think this is fine as long as we follow the next point you've made:

@lucaburgio I do believe we should align our releases more with semver though. So for example, rather than the new website being a "6.0" release (which would be a major change, and indicate a breaking change in the packages), we would just make it a "5.X" release (which would be a minor change, and indicate new features but nothing breaking).

And pretty much any new icons from here on would be "5.X" releases, until we introduce a breaking change in any of the libraries, at which point it would be "6.X"

(Also a good example for a breaking change would be the renaming of a icon, something which has already occurred in the past)

In this sense, we should look into unifying the changelog where we provide clear information about the changes for each package.

I also believe, with the way the packages are generated, if we introduced a breaking change in one of them we would likely be introducing a breaking change in all of them, so the discussion mentioned above might be moot.

True 😃

Point being, my current opinion is with the goal of the project being mass adoption, reducing friction and the possibility for confusion should be the goal, and having consistent versions reduces complexity.

I agree 👍

@lucaburgio
Copy link
Collaborator Author

Agree with you guys on all the points.
Just one doubt: Why renaming an icon would be a major change and not a minor one as adding a new icon? @sammarks @paescuj

@sammarks
Copy link
Collaborator

The only reason it would be a major change is if renaming the icon caused a breaking change. So if you're renaming some-icon to some-other-icon, and someone is importing the icon from the React library like:

import { SomeIcon } from 'iconoir-react'

The new name inside the react library would be:

import { SomeOtherIcon } from 'iconoir-react'

And it would break their existing code, unless they updated the name of the icon in their code as well. According to semver, breaking changes like this are reserved for major releases.

A new icon, on the other hand, doesn't break any existing code. It simply adds new features. So that gets a minor release.

And small updates to existing icons (like tweaking strokes, etc) would be a patch change, as that's not introducing a new feature nor introducing a breaking change.

@lucaburgio
Copy link
Collaborator Author

Totally makes sense! 👍

@paescuj
Copy link
Collaborator

paescuj commented Aug 25, 2022

Thank you, guys! 👍

To sum up:

  • Keep it that way, where all packages have the same version
  • Align releases more with SemVer
  • Unify the changelog / release notes

@sammarks
Copy link
Collaborator

The changelog should already be unified, in that we use GitHub Releases to keep track of releases, rather than a changelog file.

The only reason CHANGELOG.md exists inside the Flutter package is because, in order to release, Flutter requires one. That file is just generated from the GitHub Releases page, anyway.

@paescuj
Copy link
Collaborator

paescuj commented Aug 25, 2022

The changelog should already be unified, in that we use GitHub Releases to keep track of releases, rather than a changelog file.

The only reason CHANGELOG.md exists inside the Flutter package is because, in order to release, Flutter requires one. That file is just generated from the GitHub Releases page, anyway.

Okay, I see! 👍 I just want to make sure that the release notes are then going to contain information for each package, so the users will know whether they are affected by the update. Small example:

v5.2.0

core

  • New icon "x"

iconoir-react

  • Update of dependency "y"

iconoir-react-native

No changes

@sammarks
Copy link
Collaborator

sammarks commented Oct 5, 2022

It looks like there's nothing else for us to do on this one. Feel free to re-open if something else comes up.

@sammarks sammarks closed this as completed Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants