elm-lang / elm-package

Command line tool to share Elm libraries
BSD 3-Clause "New" or "Revised" License
213 stars 66 forks source link

Major change to native code results in only minor version number bump #265

Closed fredcy closed 7 years ago

fredcy commented 7 years ago

In https://github.com/elm-community/linear-algebra/pull/5 we modified the native JS code in a way that turned out to have a major (breaking) impact on some WebGL applications using the package, but for which elm package bump gave only a minor version number change. Because the change was minor, some of those applications were blind-sided when they automatically picked up the new version and then hit fatal errors.

So, this issue is partly to warn package maintainers of the potential problem and partly to ask if there is some way we can address the problem in elm-package. When committing the above PR I had a hunch that it was potentially more than a minor version change but I went with what elm package bump gave me.

One potential solution posed by @eeue56 is to allow something like this --

elm package bump --major

-- that would cause a major version number bump regardless of how the code changed.

In the particular case mentioned above, we forced the major version number change by making a temporary (fake) major change to the Elm code, running elm package bump, and then committing only the change to elm-package.json and throwing away the fake major change. This is a kludge and caused the generated docs to include the fake change until we submitted another major change to fix it all.

process-bot commented 7 years ago

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

evancz commented 7 years ago

What was the major change? Or even the minor change? Looking at the PR you linked, it seems like an implementation detail to me, so it should be a patch.

What was the resulting bug?

Also, having a way to force a major bump is something I know is important. It is coming eventually.

fredcy commented 7 years ago

The breaking change was to src/Native/MJS.js in this commit: https://github.com/elm-community/linear-algebra/pull/5/commits/7c8d96a6a622b94d654430f6232b66d41ed63991

That change led to this issue in WebGL: https://github.com/elm-community/linear-algebra/issues/8

The standard elm WebGL examples did not manifest the problem, but several more complicated apps such as https://github.com/w0rm/elm-mogee did.

eeue56 commented 7 years ago

It was a major change as it changed the underlying values from 32 bit to 64bit

evancz commented 7 years ago

I have more thoughts on this particular case. I don't agree with this analysis.

Putting that aside, the key recommendation is already suggested by https://github.com/elm-lang/elm-package/issues/165 and I understand why that is an important thing to have.