bitprophet / releases

A powerful Sphinx changelog-generating extension.
http://releases.readthedocs.io/
BSD 2-Clause "Simplified" License
176 stars 41 forks source link

Support for crossing major-version boundaries #45

Closed bitprophet closed 8 years ago

bitprophet commented 8 years ago

I.e. right now, in Paramiko, going from 1.x to 2.x results in: surprise! there's no 2.x displayed anywhere!

Clearly the parse and/or organization logic breaks down in some way, which isn't too surprising, I'm sure there's a lot of assumptions baked in aimed squarely at iterating over minor releases only.

Fixing this may end up solving (fully or partly) #19 and #36.

Remaining todo

bitprophet commented 8 years ago

Rambling thoughts to self: This functionality may actually be conceptually incompatible with how Releases currently works!


Consider the following scenario, where a project was on a stable 1.5 line, gets some bugfixes, adds some new backwards compatible features for release as 1.6, and also adds some backwards incompatible features for release as 2.0.

They then have a release day, putting out all 3 releases (1.5 bugfix, 1.6 feature, 2.0 backwards incompat feature):

* :release:`2.0.0 <2016-04-25>`
* :release:`1.6.0 <2016-04-25>`
* :release:`1.5.1 <2016-04-25>`
* :feature:`123` Super big huge backwards incompatible feature!
* :feature:`99` Regular feature!
* :bug:`101` Oops, a regular ol' bug.
* :release:`1.5.0 <2016-01-25>`
<prehistory here>

What should happen in this case, re: which "buckets" each item goes into? Currently we do roughly the following:

So, given the above changelog, again ignoring 2.0.0, you'd end up with:

Great. So what happens when we do want to consider 2.0?


In a super naive setup, where major releases follow from minor ones but not at the same time (i.e. if we dropped 1.6.0 from the example) things appear to "work" - but only insofar as it's 2.0.0 that gets features 123 and 99.

Conversely, what happens in the full example is that 1.6.0 "eats" both features out of unreleased_features, leaving it empty when 2.0.0 is encountered. Whoops! (And empty releases don't really count, so they don't get displayed. Bit of a corner case...)

Now, some (maybe even most) projects may happen to fit this scenario, but there's nothing about good versioning practices that says you can't continue putting out 1.x feature releases after 2.0.0.

Additionally, there's the issue of bugs: in a 1.x-and-2.x world, where 2.x is backwards incompatible from 1.x (potentially even a full rewrite!) you can't naively assume all bugs encountered recently belong in both e.g. 1.5.3 and 2.0.1. (Frustratingly, of course, you also can't assume none of them do, either - only in a full-rewrite scenario is that likely the case.)


So we clearly need either some serious annotation (if we continue "one timeline rules them all") or consider reorganization somehow (perhaps multiple changelog files and/or sections).

Annotation seems like it'd get annoying fast:

Splitting into multiple sections or files (doesn't matter which, I don't think; either way we'd just be merging them together in the end) allows for easier separation, but means the entire raison d'etre of this library is nullified:


tl;dr kind of a classic RDBMS schema (de)normalization problem...

bitprophet commented 8 years ago

More random ideas:

bitprophet commented 8 years ago

Related: I totally forgot we already added a "line" field to the mini-spec; it's currently intended for preventing bugs from going "too far back", so e.g. :bug:57 (1.2+) means "only put this bug into release line buckets for 1.2, 1.3 and so forth - don't put it in 1.0 or 1.1, it doesn't apply there".

Provided the rest of the system is overhauled as above re: major families (including "default to latest major family"), this field could fill our needs?

bitprophet commented 8 years ago

Yea, semantic_version is something I've used in my Invocations packaging module and it looks like it'll work well for this (+ other spots in here too, honestly, there's bits where we manually parse and should just use semantic_version.Version instead).

Specifically, it has a Spec class that parses strings like the above (>=1.18,<2) and can do various logic things to test if a given Version falls within a Spec or not.

One minor quibble is you can't create Versions to represent release lines (Version('2.1') is invalid) and thus can't answer questions like "which release line buckets should this issue-with-a-spec go into?". Hopefully work-around-able or e.g. subclassable.

EDIT: seems like we were already using distutils.LooseVersion for some of this; I'd bet semantic_version is more useful but will have to see.

bitprophet commented 8 years ago

NOTE: it may still be worthwhile to offer both "versions" of this, i.e. not just annotations but also "one changelog source per major line" (with the config activating/selecting this, also specifying e.g. "1.x is in changelog_1.rst, 2.x is in changelog_2.rst" so each is parsed with different rules). I think this could be a lot nicer for projects with near-100%-incompatible 2.x lines, who'd otherwise have to annotate literally every 1.x (or 2.x depending) related issue.

Good thing is, it should be easy enough to add that after the 1st variant of this feature lands, if and only if it seems truly necessary.

bitprophet commented 8 years ago

ANOTHER NOTE: while I haven't given it serious thought, I am pretty sure all annotations (i.e. major and backported) could be effected by this newer version-spec-using scheme.

E.g. a major bug inserted prior to a 1.1.0 release could be defined instead as :bug:123 1.1+ to keep it out of a 1.0.x bugfix release. (Though I think this would require the logic to change such that annotations override all other rules...) Not sure if this is actually better from a user standpoint though, and again, haven't thought it through all the way, it might not actually be 100% equivalent.

Something to ponder for our own 2.0 tho.

bitprophet commented 8 years ago

Sweet, turns out Version can do partial versions if I...say partial=True. Dunno how I missed that earlier. No workarounds necessary; works great, e.g. Spec('<2.0').filter([Version('1.1', partial=True), Version('1.2', partial=True), Version('2.0', partial=True), Version('2.1', partial=True)]) correctly yields an iterable just containing 1.1 and 1.2.

MinchinWeb commented 8 years ago

What about using :bug:57 (1.6+) to denote versions <=1.6, >2.0 and :bug:57 (1.6++) (double plus) to denote versions <=1.6 including 2.0, 3.0, etc?

That said, as I write this and re-read your comments, using the system already in place elsewhere (like used for setup.py) seems like a bright idea because then it's one less thing to learn.

bitprophet commented 8 years ago

Yea, I specifically wanted to avoid coming up with a new system when the problem domain is already well mapped out :) semantic_version is working well so far.

Have made more progress on this today, running into some implementation hijinx for corner cases, but I think I have a solution in sight.

MinchinWeb commented 8 years ago

Would it also make sense to have a label that denotes a breaking change, similar to a "major" bug? So make your sample changelog from above read:

* :release:`2.0.0 <2016-04-25>`
* :release:`1.6.0 <2016-04-25>`
* :release:`1.5.1 <2016-04-25>`
* :feature:`123 breaking` Super big huge backwards incompatible feature!
* :feature:`99` Regular feature!
* :bug:`101` Oops, a regular ol' bug.
* :release:`1.5.0 <2016-01-25>`
<prehistory here>
bitprophet commented 8 years ago

I've been doing that just by putting a .. warning:: block inside the descriptions for such features. In terms of it indicating how things get rolled up, I'm not sure if it helps any more than using >=2.0/2.0+ would in the same position.

I'll be pushing an update soonish once I am happy with the in-progress implementation but it's basically as described earlier - features just prior to a block of releases containing 2.0.0, and of course any afterwards, would default to "belonging" to 2.0 unless otherwise specified. (So you wouldn't even want to use 2.0+ in the scenario we're discussing - instead, you'd mark any "1.0 but not 2.0" features as being <2.0.)

Said update includes a bunch more explicit examples in the concepts docs, similar to the ones there now.

bitprophet commented 8 years ago

Whee, back to 100% green on the test suite now, including the first meaty test re: new functionality.

Now to fill in the rest of the new skel tests & hope they all 'just work' given the overhaul. And address some TODO's.

bitprophet commented 8 years ago

Surfaced another handful of thorny corner/edge cases while dealing with rest of new tests (& in examining how the changes affected our own changelog). Finally got those all nailed down too.

Will scan for any critical-level TODOs (the rest can wait, I did not anticipate this taking nearly 3 full days of hacking...) and then start applying to Paramiko's upcoming changelog (esp after merging #731 and #733) as a final test.

bitprophet commented 8 years ago

Started poking Paramiko's changelog, some wrinkles or potential ones:

bitprophet commented 8 years ago

Huh yea looking at the backported stuff, there is actually a regression with that, they're no longer being put in feature release buckets at all. Likely due to how things were cleaned up into is_buglike/is_featurelike. (See #50 as well, this probably is how things should be, but we can't actually change it now w/o breaking things for users.)

bitprophet commented 8 years ago

That part is easily enough fixed.

The previous issue, less so. Core problem here is interplay between "featurelike issues with no spec automatically get assigned to upcoming major releases" and "you can't specify both backported and a spec":

I experimented slightly with letting the specs imply backported behavior (especially as I mentioned earlier in this ticket that we could eventually replace the other keywords entirely by doing so) and this kinda works but it's not compatible with how the feature has been developed & documented so far - i.e. saying :feature:123 (<2.0) to keep something pinned to 1.17. Poof, that spec would then imply backporting as it matches all release lines prior to 2.0; which was not the intent of that example.

So I see two ways out of this:

bitprophet commented 8 years ago

Finally happy with how Paramiko's changelog has turned out. JFC what a boondoggle.