WIP: Optionally omit the "g" for "git describe" #154
Conversation
Previously, three git commands were used to gather all required info. Now all info is gathered from a single 'git describe' call and only if that fails fallbacks are used.
| @@ -4,7 +4,7 @@ | |||
| import warnings | |||
|
|
|||
| FILES_COMMAND = 'git ls-files' | |||
| DEFAULT_DESCRIBE = 'git describe --tags --long --match *.*' | |||
| DEFAULT_DESCRIBE = 'git describe --tags --long --dirty --match *.*' | |||
There was a problem hiding this comment.
does this work consistently, i very vaugely recall trouble with some older git versions that still may be in some distros
There was a problem hiding this comment.
At least I've never had any issues with it.
There was a problem hiding this comment.
thats ok, but "works for me" is not a metric end-users appreciate ^^
for examples see #134
There was a problem hiding this comment.
I seem to recall the dirty flag was to distinguish a working directory with uncommitted changes from a clean working copy. I'm pretty sure you want the flag.
Agreed that this change doesn't belong in this request. If you want to make this change, propose it separately and trace its origin to prove that it's no longer needed (or in which conditions that's true).
There was a problem hiding this comment.
Can you elaborate what exactly you mean by "trace its origin"?
There was a problem hiding this comment.
I mean find when that change was made and why, to try to confirm its purpose. Perhaps using a blame tool or bisecting the changelog or searching the project ticket history.
There was a problem hiding this comment.
I mean find when that change was made and why, to try to confirm its purpose. Perhaps using a blame tool or bisecting the changelog or searching the project ticket history.
There was a problem hiding this comment.
I wasn't understanding @RonnyPfannschmidt in the way that --dirty already used to be there, but did cause problems, and thus was removed. I thought that was just a general remark. But I'll investigate.
There was a problem hiding this comment.
I think I misread the diff. Yeah, Ronnys comment is probably more meaningful.
There was a problem hiding this comment.
Turns out --dirty indeed already was there: 929a5d7#diff-7a9c0c3ec91c440b46ba9f3506777bf0L7. Also see my PR #156.
|
note that i proposed the other way around specifically because i perceived it as more clean because now it removes the g at a lower level scm specific function and would have to conditionally add it back while no longer knowing the scm without looking again from my pov thats layering of band-aids and not desirable as it introduces more points of fragility that being said, i did think a bit and i am perfectly fine with releasing a 2.0 for this change as is without bringing back g on that note i am also open for a short discussion on ensuring each scm gets its own node prefix and how that should look |
Good point. I'll change it, but probably in a separate PR.
How about simply taking the first letter of the respective command line tool name ( |
|
using g/h sounds good, both are outside hex |
|
Good. So I'll eventually file a PR that adds the "h" prefix for hg, and removes the global "n" prefix. Closing this PR as it's deprecated now. |
|
👍 |
Following the discussion started here I came up with some basic changes that could lead into the direction of optionally omittin the "g" for "git describe".
The first commit is just a more or less unrelated optimization to do fewer Git calls.
The second commit removes the "g" prefix in the lower level functions by default. This will currently break things, so my idea is to optionally add the "g" back in one of the higher level functions. I'd rather do it that way than the other way around, because in the long run the "g" prefix should be dropped altogether.
Even if this is WIP and not ready yet, I'm posting this here to get some early feedback.