Nico Huber posted comments on this change.
Patch set 1:
Answered some inline comments. Though, I really think we should move
any further discussion to changes based on the current staging branch.
We'd have to sync staging with the solution for stable anyway and
reviewing/discussing will be much easier on simple changes than a
everything-at-once commit.
Stefan, please chunk a diff of your current version against staging
into smaller commits and push them for review to staging.
(5 comments)
With this patch the version string includes the last reachable git tag,
number of commits since said tag in the upstream branches (if any),
the name of said upstream branch, number of commits since that branch
(if any), and the shortened git hash.
Apart from the additional complexity of the get_revision script
I don't really see any downsides.
Yeah, it's doable ofc. But you already proved that it's pretty
hard to put it together into a sound script. Just come up with
something that makes it through a review if you want to spend
more time on it.
Patch Set #1, Line 1377: x;n;n;s/^set$$//;t;x;p
One question: why the '{}' around 'b'?
Doesn't seem to be needed. I wasn't sure if something like //p;p means
//{p;};p or //{p;p;} (semantics of ; are not really defined, but POSIX
implicitly specifies the former). Though I'd argue that with the braces
you can see more clearly that the ;x;p at the end is separate from the
address. Wouldn't be necessary if you write the program in multiple
lines, e.g.:
x
n
n
/^set$/b
x
p
With that written down, how about 'x;n;n;{/^set$$/b;};x;p;'? ;)
I've just learned that there should be a semicolon before the closing
brace:
The <right-brace> shall be preceded by a <newline> or
<semicolon> (before any optional <blank> characters
preceding the <right-brace>).
GNU sed doesn't care...
# We need to update our upstream information sometimes so that we can create detailed version information.
# To that end the code below fetches parts of the upstream repository via https.
# This takes about one second or less under normal circumstances.
No, we break at the latest at a 112 character limit because that's what was
I didn't take part in that discussion, can look it up, though, if
necessary. I'd argue that the 112 column limit makes much more
sense for code (that usually is indented and has a shorter effec-
tive line length) than it does for running text that (maybe) some-
body is supposed to read (it's really much harder to read with
increasing line length).
Patch Set #1, Line 269: [ -n "$upstream_branch" ]
No, there might be upstream branches that are neither stable nor staging (e
I was talking about the code flow: `upstream_branch` is only set
together with `cnt_tag2upstream_branch` above. So the former can't
be unset if the latter is set.
File util/git-hooks/install.sh:
Patch Set #1, Line 17: ../../
No, IIRC, the idea is to make these relative links because else they get in
Yeah, and IIRC, we didn't change that. `git rev-parse --show-cdup`
is supposed to output a relative path.
To view, visit change 19206. To unsubscribe, visit settings.