search

FluxML / GeometricFlux.jl

Geometric Deep Learning for Flux
https://fluxml.ai/GeometricFlux.jl/stable/
MIT License
348 stars 30 forks source link

This package does not respect semantic versioning #266

Closed CarloLucibello closed 2 years ago

CarloLucibello commented 2 years ago

@yuehhua you just tagged a patch release, 0.8.1, after merging a PR (https://github.com/FluxML/GeometricFlux.jl/pull/254) with breaking changes. This is very bad.

yuehhua commented 2 years ago

Sorry, I didn't notice that. How can I fix it? Can I just untag v0.8.1 and tag v0.9.0?

CarloLucibello commented 2 years ago

It's not the first time this happened and caused breakage. I think this package and how it is maintained are not up to the standards of the FluxML org.

I don't know how to revert a tag.

yuehhua commented 2 years ago

I just tag v0.8.2 to revert changes in v0.8.1, and the changes are tagged as v0.9.0.

CarloLucibello commented 2 years ago

tests are failing on Julia 1.6 for the v0.9.0 tagged commit https://github.com/FluxML/GeometricFlux.jl/runs/5265036135?check_suite_focus=true

yuehhua commented 2 years ago

It seems to fail on calculating gradient of map function, but passed in v1.7.

CarloLucibello commented 2 years ago

Did you just introduce another breaking change in #269 changing the exported VariationalEncoder to VariationalGraphEncoder and tagged a patch release?

yuehhua commented 2 years ago

I did introduce another breaking change, but I didn't tag a patch release. I just notice that v0.8.2 is not tagged and I tagged it manually on the same commit as v0.8.0.

yuehhua commented 2 years ago

I am curious. I know that I often miss the breaking changes introduced in PR and forget to tag a release, instead of patch. I come out with two questions:

  1. What's the full definition for breaking changes? Does it include only exported APIs or even non-exported APIs? Suppose I extend APIs by adding new arguments to a function with default value. It changes the API itself but may not break the current usage. Is it still a breaking?
  2. Is there a way to check any breaking changes before tagging a version? How do you check it?
CarloLucibello commented 2 years ago

There is no universally agreed upon definition of what is breaking for julia library changes, but I think everyone agrees on the following. The basic idea is that downstream projects should be able to do pkg> up and if they have GeometrixFlux="0.7.0" in their Project.toml their code should keep working on any 0.7.x.

I think that everyone agrees that the following is considered breaking:

Adding extra keyword arguments to a function is not breaking if the old code will continue working in the same way. Generally you can change the API provided old code will work the same way.

Some people argue that exporting new symbols should be considered breaking since it can conflict with symbols exported by other packages, but most packages are not so strict and don't consider this breaking, just pay attention and don't export very common names.