ietf-tools / semver-action

GitHub Action to calculate the next release version based on conventional commits
BSD 3-Clause "New" or "Revised" License
56 stars 22 forks source link

skipInvalidTags required for monorepos using prefix #39

Closed johnburbridge closed 9 months ago

johnburbridge commented 9 months ago

I just found an interesting quirk which I think would benefit from either a doc update or a patch. I'm happy to contribute a PR for either, btw.

The issue is that when using prefixes in a monorepo, unless skipInvalidTags is true or the latest tag is of the same prefix as the incoming version, then the error "Latest tag is invalid (does not conform to semver)!" is thrown, which is technically correct but odd.

Case in point:

const semver = require("semver");
const core = require("@actions/core");

const tagsList = [ { name: 'foo/v0.1.0' }, { name: 'bar/v0.1.1' }]
const prefix = 'bar/'

let skipInvalidTags = false
let idx = 0
let latestTag = ''

for (const tag of tagsList) {
  if (prefix && tag.name.indexOf(prefix) === 0) {
    tag.name = tag.name.replace(prefix, '')
  }
  if (semver.valid(tag.name)) {
    latestTag = tag
    break
  } else if (idx === 0 && !skipInvalidTags) {
    break
  }
  idx++
}

if (!latestTag) {
  return core.setFailed(skipInvalidTags ? 'None of the 10 latest tags are valid semver!' : 'Latest tag is invalid (does not conform to semver)!')
}

core.info('latestTag: ' + latestTag.name)

The above prefix doesn't match the first tag and throws the error. Technically, anything with a prefix shouldn't be valid semver (even v) but from a UX perspective it feels awkward in that semver-action allows for a prefix and then calls it invalid if used.

If skipInvalidTags is true, then everything works presumably as expected.

It would seem the solution would be to either update the description of skipInvalidTags and prefix in the docs, possibly adding an example for monorepo users, or decoupling the idx check from skipInvalidTags so that the conditions are checked independently from each other?

NGPixel commented 9 months ago

I'm not sure I understand the case where you would compare 2 completely different prefixes? Wouldn't that be 2 different "apps" that are expected to be on different versions?

johnburbridge commented 9 months ago

Correct. Different apps in the same repo with their respective versions that evolve separately.

/foo, v1.2.3 = foo/v1.2.3 /bar, v0.1.0 = bar/v0.1.0

In the sample above, if the most recent tag is foo/v0.1.0 but the job is triggered for app bar (prefix bar/) then the "Latest tags is invalid (does not conform to server)!" is thrown unless skipInvalidTags is set to true.

I assume that's intentional but it's not documented as a requirement for monorepo setup, which threw me off. Maybe I'm the odd one out but I think a README update would help others trying to do the same.

NGPixel commented 9 months ago

Fixed in v1.7.1