git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 0/2] builtin/sparse-checkout: add check-rules command

From
Elijah Newren <newren@gmail.com>
Date
Apr 1, 2023, 18:49 UTC
Message-ID
<CABPp-BHhjjO32EAzi_Deck6fxsyG=uMrvAnJLaD0e9qMtrcDEQ@mail.gmail.com>
In-Reply-To
<pull.1488.v2.git.1679903703.gitgitgadget@gmail.com>
Hi,

On Mon, Mar 27, 2023 at 12:55 AM William Sprent via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 23 quoted lines
>
> Hi
>
> This v2 addresses comments from Elijah's review comments.
>
> There's one thing worth highlighting. Elijah pointed out that the the
> "check-rules cone mode is default" test would be stronger if the test itself
> started with a 'git sparse-checkout set --no-cone' to explicitly test that
> the default interpretation of the rules passed with the '--rules-file'
> option is cone mode even though the current checkout is non-cone. I
> implemented this and it exposed that the option did not actually behave that
> way, and that the test only verified the default behaviour of a bare
> repository.
>
> I've modified the logic of the '--rules-file' option such that it defaults
> to cone mode unless combined with '--no-cone', and I've added a line to the
> documentation to make this more explicit.
>
> The alternative would be to have '--rules-file' assume non cone mode when in
> a non cone mode checkout, but I think this depends a bit on "how deprecated"
> non-cone mode is vs. how important it is to have the option behave
> consistently with 'sparse-checkout set' (which respects the current
> checkout).
Ooh, nice catch.

Given that the whole point of check-rules and --rules-file is to determine how a _different_ sparse checkout would behave, I agree with your decision here that the current sparse checkout should not affect how --rules-file functions.

Show 16 quoted lines
>
> Changes since v1:
>
>  * Explicitly state in documentation that '--rules-file' expects newline
>    separated rules.
>  * Explicitly state in documentation that '-z' does not affect the
>    '--rules-file' input.
>  * Fixup typo where 'When called with the --rules-file <file> flag' was
>    missing "flag".
>  * Fixup behaviour in 'check-rules --rules-file', such that it defaults to
>    accepting cone mode patterns when in a non cone checkout.
>  * Remember to release string buffers in 'check_rules()'.
>  * Explicitly state in documentation that '--rules-file' defaults to cone
>    mode unless combined with '--no-cone'.
>  * Better test that the default of '--rules-file' is to expect '--cone-mode'
>    by running 'check-rules' in a non-cone mode checkout.

This round addresses all my issues, and I agree with the other change made in this round. So, v2 is:

Reviewed-by: Elijah Newren <newren@gmail.com>
Thanks for sending this in, William!
Previous: William Sprent via GitGitGadgetNext: Junio C Hamano
Message 17 of 18 in “builtin/sparse-checkout: add check-rules command”
  1. 0/2 builtin/sparse-checkout: add check-rules commandWilliam Sprent via GitGitGadget, Mar 8, 2023
  2. 1/2 builtin/sparse-checkout: remove NEED_WORK_TREE flagWilliam Sprent via GitGitGadget, Mar 8, 2023
  3. Junio C HamanoMar 8, 2023
  4. Junio C HamanoMar 8, 2023
  5. Elijah NewrenMar 9, 2023
  6. Junio C HamanoMar 9, 2023
  7. 2/2 builtin/sparse-checkout: add check-rules commandWilliam Sprent via GitGitGadget, Mar 8, 2023
  8. Elijah NewrenMar 19, 2023
  9. William SprentMar 20, 2023
  10. Elijah NewrenMar 19, 2023
  11. 0/2 builtin/sparse-checkout: add check-rules commandWilliam Sprent via GitGitGadget, Mar 27, 2023
  12. 1/2 builtin/sparse-checkout: remove NEED_WORK_TREE flagWilliam Sprent via GitGitGadget, Mar 27, 2023
  13. Junio C HamanoMar 27, 2023
  14. SZEDER GáborApr 7, 2023
  15. Junio C HamanoApr 7, 2023
  16. 2/2 builtin/sparse-checkout: add check-rules commandWilliam Sprent via GitGitGadget, Mar 27, 2023
  17. Elijah NewrenApr 1, 2023
  18. Junio C HamanoApr 3, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.