Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Use git describe correctly #2572

Closed
wants to merge 1 commit into from

Conversation

ChiefORZ
Copy link

@ChiefORZ ChiefORZ commented May 6, 2020

When automatically detecting the next canary version with lerna publish --canary somehow lerna was not detecting the most recent canary version of the package or the packages automatically... This commit fixes it in this, but i dont know if something else breaks with this change.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@evocateur evocateur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a test to verify this, and I don't see any link to a reproduction that would otherwise suffice as a reason to change this. There are plenty of tests that validate the git describe command as written works reasonably well.

What exactly are you trying to fix with this change?

@@ -10,6 +10,8 @@ module.exports.sync = sync;
function getArgs(options, includeMergedTags) {
let args = [
"describe",
// return latest tag - otherwise the ref is incoorrect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "correct" you mean... detects broken tags that aren't properly annotated? There should be no situation in which Lerna itself creates release tags that are undetectable without the --tags option here.

Copy link
Contributor

@azu azu Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have met similar issue.

I've release packages and tagged git tag via GitHub API from GitHub Actions.
The tag is lightweight(non-annotated) tag.

git describe can not get lightweight(non-annotated) tag without --tags flag.

Example repository used GitHub API for tagging git tag

https://github.com/azu/lerna-monorepo-github-actions-release

In this repository, git describe show old git tag. (Probably, latest annotated git tag)

❯ git describe
v1.3.0-68-ge149829

❯ git describe --tags
v2.0.1-3-ge149829

As a result, lerna changed show 1.3.0...latest changes. (latest tag is 2.0.1)
It make lerna version --conventional-commits always suggest major update.

To use git describe --tags will fix this issue that I met.

@ChiefORZ ChiefORZ force-pushed the feature/git-describe-correctly branch from bcc1d8a to d4b1b21 Compare October 1, 2020 15:10
@evocateur evocateur changed the base branch from master to main November 8, 2020 00:40
@evocateur
Copy link
Member

Lerna is using git describe correctly for its use case. External tools need to produce correctly-annotated tags to be compatible.

@brandones
Copy link

brandones commented Oct 21, 2021

@evocateur Looks like there's about three different issues, each with a few people giving reasons why this PR is a necessary fix. The existing behavior of --canary is buggy. See #2622 and its duplicates, #2780 and #2778 .

@brandones
Copy link

You should be able to get a repository into a state such that it needs this change simply by using lerna publish --no-git-tag-version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants