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

[usability issue] Doesn't return either distance or semver #27

Open
OnkelTem opened this issue Aug 11, 2022 · 4 comments
Open

[usability issue] Doesn't return either distance or semver #27

OnkelTem opened this issue Aug 11, 2022 · 4 comments

Comments

@OnkelTem
Copy link

OnkelTem commented Aug 11, 2022

I created a simple js file in the root of my project:

const { gitDescribeSync } = require('git-describe')
const gitInfo = gitDescribeSync()
console.log(gitInfo)

which returns:

{
  dirty: true,
  raw: 'd6d7fc7-dirty',
  hash: 'd6d7fc7',
  distance: null,
  tag: null,
  semver: null,
  suffix: 'd6d7fc7-dirty',
  semverString: null,
  toString: [Function (anonymous)]
}

like it couldn't acquire either of those. But git describe w/o params properly returns:

0.4.1-4-gd6d7fc7
@OnkelTem
Copy link
Author

Ok, I've resolved the issue after investigating source code. We don't use v prefix for our versions, while git-describe module uses --match option equal to 'v[0-9]*' which filtered our tags out.

I would recommend not using any mask by default.

@OnkelTem OnkelTem changed the title [bug] Doesn't return either distance or semver [usability issue] Doesn't return either distance or semver Aug 11, 2022
@TimothyJones
Copy link
Collaborator

I think the reason for the default mask is that several popular tools produce versions in that format. It could perhaps be better highlighted, or intelligently fall back by default.

@tvdstaaij
Copy link
Owner

The reason is that this was initially developed for a specific use case, and the defaults were chosen to fit that particular use case. Publishing it for reuse by others was more of an afterthought. Over time I did add options and made some tweaks to make it more suitable for other use cases as well, but the defaults are not optimized for the broadest possible audience.

So basically it boils down to a legacy issue. I wouldn't mind changing this, but I don't think it's worth breaking compatibility over. Maybe it could be bundled if there is ever a more fundamental reason for a semver-major update. @TimothyJones I'm not sure how an intelligent fallback would work, do you have an idea of how that could be achieved without breaking compatibility with code that relies on this default?

@TimothyJones
Copy link
Collaborator

I think bringing in a fallback would be a breaking change. What I was thinking was it might be nice if the default was vX.Y.Z, and it fell back to X.Y.Z only if it didn't find anything, but I'm not actually sure that's a good idea - it's good if software just works, but it's not good if the behaviour is too surprising or might result in the wrong output.

I think the ideal might be a warning of "hey we didn't find anything, but we did find X.Y.Z without the prefix", but I'm not sure how that warning would best be presented - printing to standard error won't be what everyone wants, and throwing an error seems a bit harsh. We could add a debug option to print warnings, or make it throw and add a force option to make it ignore the warnings - but both those options feel more complex than the benefit we'd get.

I made the claim that v is a good default because it's the default prefix on other versioning tools, so I did a quick survey of the ones I'm familiar with:

All of these include the v prefix by default. There probably are tools that don't, but I don't personally know any.

The semver spec doesn't mention any prefix, but I think the reason the v prefix is the conventional default on tags is to make it clear that the tag is intended to represent a 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

No branches or pull requests

3 participants