mbox series

[v2,0/5] Fixes for Documentation/MyFirstObjectWalk.txt

Message ID cover.1710840596.git.dirk@gouders.net (mailing list archive)
Headers show
Series Fixes for Documentation/MyFirstObjectWalk.txt | expand

Message

Dirk Gouders March 19, 2024, 11:23 a.m. UTC
The second spin for this series.
---
Changes since v1:
* Added Emily to Cc in the hope for a review
* Remove superfluous tags from [1/5] and [3/5]
* Replace bashism `|&` by `2>&1 |` in [5/5]
---
Dirk Gouders (5):
  MyFirstObjectWalk: use additional arg in config_fn_t
  MyFirstObjectWalk: fix misspelled "builtins/"
  MyFirstObjectWalk: fix filtered object walk
  MyFirstObjectWalk: fix description for counting omitted objects
  MyFirstObjectWalk: add stderr to pipe processing

 Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Range-diff against v1:
1:  15b74566e0 ! 1:  babf04295e MyFirstObjectWalk: use additional arg in config_fn_t
    @@ Commit message
         Fix those calls and the example git_walken_config() to use
         that additional argument.
     
    -    Fixes: a4e7e317 (config: add ctx arg to config_fn_t)
    -    Cc: Glen Choo <glencbz@gmail.com>
         Signed-off-by: Dirk Gouders <dirk@gouders.net>
     
      ## Documentation/MyFirstObjectWalk.txt ##
2:  c1ac705840 = 2:  ab0b820df7 MyFirstObjectWalk: fix misspelled "builtins/"
3:  0f67a161ef ! 3:  fac6886af3 MyFirstObjectWalk: fix filtered object walk
    @@ Commit message
         rev->filter to parse_list_objects_filter() in accordance to
         such a call in revisions.c, for example.
     
    -    Fixes: f0d2f849 (MyFirstObjectWalk: update recommended usage)
    -    Cc: Derrick Stolee <stolee@gmail.com>
         Signed-off-by: Dirk Gouders <dirk@gouders.net>
     
      ## Documentation/MyFirstObjectWalk.txt ##
4:  637070dd48 = 4:  33a1845889 MyFirstObjectWalk: fix description for counting omitted objects
5:  a2d30eff21 ! 5:  64c36dbf16 MyFirstObjectWalk: add stderr to pipe processing
    @@ Commit message
         trace messages are sent to stderr if GIT_TRACE is set to '1', so those
         commands do not produce the described results.
     
    -    Fix this by using the operator '|&' to additionally connect stderr to
    -    stdin of the latter command.
    +    Fix this by redirecting stderr to stdout prior to the pipe operator
    +    to additionally connect stderr to stdin of the latter command.
     
         Signed-off-by: Dirk Gouders <dirk@gouders.net>
     
    @@ Documentation/MyFirstObjectWalk.txt: those lines without having to recompile.
      
      ----
     -$ GIT_TRACE=1 ./bin-wrappers/git walken | head -n 10
    -+$ GIT_TRACE=1 ./bin-wrappers/git walken |& head -n 10
    ++$ GIT_TRACE=1 ./bin-wrappers/git walken 2>&1 | head -n 10
      ----
      
      Take a look at the top commit with `git show` and the object ID you printed; it
    @@ Documentation/MyFirstObjectWalk.txt: of the first handful:
      ----
      $ make
     -$ GIT_TRACE=1 ./bin-wrappers git walken | tail -n 10
    -+$ GIT_TRACE=1 ./bin-wrappers git walken |& tail -n 10
    ++$ GIT_TRACE=1 ./bin-wrappers git walken 2>&1 | tail -n 10
      ----
      
      The last commit object given should have the same OID as the one we saw at the

Comments

Kyle Lippincott March 23, 2024, 10 p.m. UTC | #1
On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:
> The second spin for this series.
> ---
> Changes since v1:
> * Added Emily to Cc in the hope for a review
> * Remove superfluous tags from [1/5] and [3/5]
> * Replace bashism `|&` by `2>&1 |` in [5/5]
> ---
> Dirk Gouders (5):
>   MyFirstObjectWalk: use additional arg in config_fn_t
>   MyFirstObjectWalk: fix misspelled "builtins/"
>   MyFirstObjectWalk: fix filtered object walk
>   MyFirstObjectWalk: fix description for counting omitted objects
>   MyFirstObjectWalk: add stderr to pipe processing
> 
>  Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------
>  1 file changed, 20 insertions(+), 16 deletions(-)

Aside from the small comments on 4 and 5, series looks good to me, thanks for
working on this.
Dirk Gouders March 23, 2024, 11:06 p.m. UTC | #2
Kyle Lippincott <spectral@google.com> writes:

> On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:
>> The second spin for this series.
>> ---
>> Changes since v1:
>> * Added Emily to Cc in the hope for a review
>> * Remove superfluous tags from [1/5] and [3/5]
>> * Replace bashism `|&` by `2>&1 |` in [5/5]
>> ---
>> Dirk Gouders (5):
>>   MyFirstObjectWalk: use additional arg in config_fn_t
>>   MyFirstObjectWalk: fix misspelled "builtins/"
>>   MyFirstObjectWalk: fix filtered object walk
>>   MyFirstObjectWalk: fix description for counting omitted objects
>>   MyFirstObjectWalk: add stderr to pipe processing
>> 
>>  Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------
>>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> Aside from the small comments on 4 and 5, series looks good to me, thanks for
> working on this.

Thanks for the review -- especially for the detailed explanation and
suggestions on 4.

Dirk
Junio C Hamano March 24, 2024, 2:20 a.m. UTC | #3
Dirk Gouders <dirk@gouders.net> writes:

> Kyle Lippincott <spectral@google.com> writes:
>
>> On Tue, Mar 19, 2024 at 12:23:10PM +0100, Dirk Gouders wrote:
>>> The second spin for this series.
>>> ---
>>> Changes since v1:
>>> * Added Emily to Cc in the hope for a review
>>> * Remove superfluous tags from [1/5] and [3/5]
>>> * Replace bashism `|&` by `2>&1 |` in [5/5]
>>> ---
>>> Dirk Gouders (5):
>>>   MyFirstObjectWalk: use additional arg in config_fn_t
>>>   MyFirstObjectWalk: fix misspelled "builtins/"
>>>   MyFirstObjectWalk: fix filtered object walk
>>>   MyFirstObjectWalk: fix description for counting omitted objects
>>>   MyFirstObjectWalk: add stderr to pipe processing
>>> 
>>>  Documentation/MyFirstObjectWalk.txt | 36 ++++++++++++++++-------------
>>>  1 file changed, 20 insertions(+), 16 deletions(-)
>>
>> Aside from the small comments on 4 and 5, series looks good to me, thanks for
>> working on this.
>
> Thanks for the review -- especially for the detailed explanation and
> suggestions on 4.

Yeah, I too liked the comments on [4/5].  Thanks for working well
together.