Oceananigans.jl icon indicating copy to clipboard operation
Oceananigans.jl copied to clipboard

Highlighting important breaking changes in releases

Open tomchor opened this issue 2 years ago • 5 comments

Not sure if this is the best place to post this, but I was thinking it would be good if, every time there is a breaking release (or a release that's particularly significant even if it's non-breaking), there is a small text in the release indicating the important/breaking features. (Or maybe even something on slack?)

This is something that has implicitly come up in a few conversations I've had with Oceananigans users that don't also do development (and thus don't keep track of PRs, etc.). The advantages that I can see are:

  • Users can have an idea if their code will break with updating, which might prompt them to update more often; (I think in practice lots of users don't update their code regularly since there's a looming possibility of their code to stop running and them having to deal with errors.)
  • If their code will break, users will have an idea as to what they should change in order to fix it (as opposed to running it and fixing the code error-by-error).

The downside is that it requires someone to manually write something in most releases. But I think this is easy enough that it shouldn't be a problem.

PS: Btw I should say, I mentioned non-developer users above, but I very much include myself in the group that would benefit from this. There's often a lot going on in Oceananigans and it's easy to get overwhelmed with research and lose track of important PRs. For example v0.83.0 just dropped, implying a breaking change, and I cannot figure out which was the breaking change.

Curious to hear everyone's thoughts.

tomchor avatar Jun 16 '23 14:06 tomchor

I agree. Perhaps we could link releases that introduce breaking changes to a specific PR that's dedicated to updating the version and detailing the API breaking change. This might be tedious work (for us) because it involves tracking down all API changes in past PRs, but it might be very beneficial for users

simone-silvestri avatar Jun 16 '23 19:06 simone-silvestri

I agree with this --- though this something similar is already our policy. PRs that have breaking changes are supposed to update the minor version, and also describe the PR in words in the top comment, which certainly includes describing a breaking change. It's true that we don't achieve this, however. We could be stricter in PR reviews?

To make it easier to find the specific PRs that has the change which updates the minor version within release notes, we could have a policy to put the new version in the name of the PR, ie preface the PR with

(0.83.0) Title of the PR

That way one can easily find the relevant PR if there is a long list of PRs within a particular release, only one of which is important. (Note this information can also be obtained via git blame on Project.toml.)

One concern I have is that we struggle to enforce conventions already, like style conventions in source code, branch naming conventions, etc. For example, issues should be labeled --- this issue itself needs a label!

What do we think about working on making PR titles more descriptive and helpful, rather than copying information that I think should be in PRs into release notes? It's nice to encourage people to look at PRs too; we can have conversations there (unlike release notes). As far as breaking changes go, by definition they should be encapsulated in a single PR so I think this will succeed. Another policy change that could help is to require two approving reviews (rather than just one), and to also informally try to be more strict about approving PRs.

Users can have an idea if their code will break with updating, which might prompt them to update more often; (I think in practice lots of users don't update their code regularly since there's a looming possibility of their code to stop running and them having to deal with errors.)

Isn't trying an update and running some code / some kind of test the fastest and most effective way to deciding whether to update some package?

If their code will break, users will have an idea as to what they should change in order to fix it (as opposed to running it and fixing the code error-by-error).

I'm not sure this is feasible.

To use git blame to find the PR, navigate to Project.toml and toggle the Blame key at the top left of the file:

image

In this case the breaking change is supposedly on #2912.

I started a discussion here: https://github.com/CliMA/Oceananigans.jl/commit/4ff41ab95c1313715e91863a9b3bc2dc8ab1d458

glwagner avatar Jun 17 '23 05:06 glwagner

@glwagner @simone-silvestri I took the liberty to modifying the release notes for v0.84.0 to illustrate what my idea here was. @glwagner had already included something but I extended it a bit.

Basically a user reading the release notes know quickly what changed: compression kwarg is gone and is replaced by deflatelevel (which also link directly to https://github.com/CliMA/Oceananigans.jl/pull/3153 for more info) and there's a quick summary of changes to VectorInvariant also with a link to https://github.com/CliMA/Oceananigans.jl/pull/3091.

@simone-silvestri reading the description of https://github.com/CliMA/Oceananigans.jl/pull/3091 I couldn't understand what was breaking about that change. Seems like it just added more features/kwargs, with isn't necessarily breaking. Feel free to modify my description to clairfy.

I also included a "highlights" section, which quickly points to a user what new features are available. At this point it only references to https://github.com/CliMA/Oceananigans.jl/pull/3145, but I think https://github.com/CliMA/Oceananigans.jl/pull/3145 probably should be there too; I just don't understand it well enough to be sure and summarize everything with a simple sentence. Feel free to add.

This "highlights" section may be a bit repetitive, since similar information is contained in the list of PRs at the bottom. But my rationale is that most of those PRs don't really affect an average user (for example the doc examples parallelization is super important, but not relevant to users), so most people probably won't read their description.

In any case, feel free to modify or undo what I wrote there. I did it mostly for illustration purposes :)

tomchor avatar Jun 29 '23 22:06 tomchor

good work, I have modified the Breaking and will change also some of the highlights

simone-silvestri avatar Jun 29 '23 23:06 simone-silvestri

The PR titles should contain all information, so "modifying the release notes" should really just be copy/paste only. If you find yourself modifying notes with more than copy/paste, the problem is that the PR was not correctly titled.

glwagner avatar Jul 05 '23 16:07 glwagner