Re: [PATCH v3 2/3] pretty: add tag option to %(describe)

2 messages, 2 authors, 2021-10-29 · open the first message on its own page

Re: [PATCH v3 2/3] pretty: add tag option to %(describe)

From: Junio C Hamano <hidden>
Date: 2021-10-29 20:18:21

Eli Schwartz [off-list ref] writes:
 +
+** 'tags[=<BOOL>]': Instead of only considering annotated tags,
+   consider lightweight tags as well.
This part contradicts what Jean-Noël's df34a41f is trying to
achieve, which can be seen in these hunks from it:

    @@ -273,12 +273,12 @@ endif::git-rev-list[]
                              If any option is provided multiple times the
                              last occurrence wins.
     +
    -The boolean options accept an optional value `[=<BOOL>]`. The values
    +The boolean options accept an optional value `[=<value>]`. The values
     `true`, `false`, `on`, `off` etc. are all accepted. See the "boolean"
     sub-section in "EXAMPLES" in linkgit:git-config[1]. If a boolean
     option is given with no value, it's enabled.
     +
    -** 'key=<K>': only show trailers with specified key. Matching is done
    +** 'key=<key>': only show trailers with specified <key>. Matching is done
        case-insensitively and trailing colon is optional. If option is
        given multiple times trailer lines matching any of the keys are
        shown. This option automatically enables the `only` option so that
    @@ -286,25 +286,25 @@ option is given with no value, it's enabled.
        desired it can be disabled with `only=false`.  E.g.,
        `%(trailers:key=Reviewed-by)` shows trailer lines with key
        `Reviewed-by`.
    -** 'only[=<BOOL>]': select whether non-trailer lines from the trailer
    +** 'only[=<bool-value>]': select whether non-trailer lines from the trailer
        block should be included.
    -** 'separator=<SEP>': specify a separator inserted between trailer
    +** 'separator=<sep>': specify a separator inserted between trailer
     ...


So, let's instead use

    tags[=<bool-value>]: Instead of only considering ...

i.e. lowercase, with -value suffix.

Thanks.

Re: [PATCH v3 2/3] pretty: add tag option to %(describe)

From: Eli Schwartz <hidden>
Date: 2021-10-29 21:14:23

On 10/29/21 4:18 PM, Junio C Hamano wrote:
Eli Schwartz [off-list ref] writes:
quoted
 +
+** 'tags[=<BOOL>]': Instead of only considering annotated tags,
+   consider lightweight tags as well.
This part contradicts what Jean-Noël's df34a41f is trying to
achieve, which can be seen in these hunks from it:

[...]

So, let's instead use

    tags[=<bool-value>]: Instead of only considering ...

i.e. lowercase, with -value suffix.

An interesting change. I can use that description style, sure. Though I
will note the commit message for it talks a lot about replacing spaces
with hyphens, and very little about consolidating on case *or* using
different language such as:


-* 'format:<string>'
+* 'format:<format-string>'


I also assume that it's fine for my patches to be inconsistent with the
base commit, as it's expected df34a41f or some revision of it will be
merged around the same time?


-- 
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help