substrait-io / substrait-java

Apache License 2.0
74 stars 70 forks source link

ci: pin conventional-changelog version #175

Closed vbarua closed 10 months ago

vbarua commented 10 months ago

xref: https://github.com/semantic-release/semantic-release/issues/2929

ibis-substrait was affected by this as well: https://github.com/ibis-project/ibis-substrait/pull/742

gforsyth commented 10 months ago

You can also run the dry_run locally and it should spit out the expected changelog since the last release (for a local check)

gforsyth commented 10 months ago

looking good:

## 1.0.0 (https:/tmp/tmp.xHGP1hYI1L/compare/v0.15.0...v1.0.0) (2023-09-06)

### ⚠ BREAKING CHANGES

    * * windowFunction expression creator now requires window bound type parameter
    * the WindowBound POJO representation has been reworked to use visitation and more closely match the spec
    * ExpressionRexConverter now requires a WindowFunctionConverter
    * * feat: convert Substrait window functions to Calcite RexOvers (#172) (7618bb8 (https:/tmp/tmp.xHGP1hYI1L/commit/7618bb82150a430c3a8a9621c28e983e66785230)), closes #172 (https:/tmp/tmp.xHGP1hYI1L/issues/172)
gforsyth commented 10 months ago

Also, just so it isn't a surprise, the BREAKING CHANGE commits means the next release will be 1.0.0

vbarua commented 10 months ago

Also, just so it isn't a surprise, the BREAKING CHANGE commits means the next release will be 1.0.0

Should it be 0.16.0 actually? I thought breaking changes were allowed pre-1.0?

vbarua commented 10 months ago

We've published breaking changes before that only bumped this up by a minor? https://github.com/substrait-io/substrait-java/releases/tag/v0.15.0

gforsyth commented 10 months ago

we might have to summon @cpcloud for his conventional commits expertise

vbarua commented 10 months ago

Hmm, it looks like we configure the commit analyser to treat breaking changes as minor version here: https://github.com/substrait-io/substrait-java/blob/4922c82cfc45de6f86e5e0388bd6a98086ca24b3/.releaserc.json#L13C1-L20C7

It looks like the dry run doesn't have that configured.

vbarua commented 10 months ago

Okay it looks like the default rules as defined in https://github.com/semantic-release/commit-analyzer/blob/6026589075daf31928b408527e9e8988c3146f47/lib/default-release-rules.js#L7C38-L7C38 includes

{ breaking: true, release: "major" },

and we use

{"breaking": true, "release": "minor"}

Going back in time, I ran a sense check by executing a dry-run on the commit before the prior chore, which is the commit before our 0.15 release.

The output was

[10:21:08 AM] [semantic-release] › ℹ  Found git tag v0.14.1 associated with version 0.14.1 on branch tmp.TlLICli74x
[10:21:08 AM] [semantic-release] › ℹ  Found 5 commits since last release
...
[10:21:08 AM] [semantic-release] › ℹ  The next release version is 1.0.0

so this appears to be a dry-run artifact.

I feel pretty confident that merging and releasing this isn't about to publish a 1.0 version.

vbarua commented 10 months ago

v.0.16.0 was correctly released https://github.com/substrait-io/substrait-java/releases/tag/v0.16.0

gforsyth commented 10 months ago

woo! nice detective work!