[RFC,0/1] check-attr: integrate with sparse-index

Message

Shuqi Liang Feb. 27, 2023, 5:05 a.m. UTC

Integrate git check-attr with sparse-index.

Only add 'check-attr pathspec inside sparse definition' yet, just want to see 
if the added tests in this patch  on the right track.
Also, I have a question about git rev-parse. I read the documentation and wanted to add a test for it, 
but I noticed that Derrick already integrated it with sparse-index last year. 
Is there anything else I can do with git rev-parse to integrate with sparse-index, or should we just leave it be? 

Thanks!

Shuqi Liang (1):
  test check-attr pathspec inside sparse definition

 builtin/check-attr.c                     |  3 +++
 t/t1092-sparse-checkout-compatibility.sh | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)


base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1

Comments

Victoria Dye Feb. 27, 2023, 5:18 p.m. UTC | #1

Shuqi Liang wrote:
> Integrate git check-attr with sparse-index.

Since you're only looking for feedback on a test (one that theoretically
passes without any sparse index changes), the title should reflect that.
Something like "t1092: add a test for check-attr".

Also, the line wrapping of the following lines is a bit strange. Please make
sure to wrap to 72 characters, as it makes responding inline easier for
reviewers. For the sake of commenting here, I've rewrapped them.

> 
> Also, I have a question about git rev-parse. I read the documentation and
> wanted to add a test for it, but I noticed that Derrick already integrated
> it with sparse-index last year. Is there anything else I can do with git
> rev-parse to integrate with sparse-index, or should we just leave it be? 

Ah, sorry about that, I forgot to remove it. I don't think there's anything
else to do there, so we can leave it be.

I also think the list on the SoC Ideas 2023 [1] is somewhat out of order
(there are definitely some easier/almost trivial ones marked as "harder"
than some more involved ones) - I noticed last year, but never got around to
updating it. I'll try to submit a pull request to update the order and
remove 'rev-parse' sometime today or tomorrow. Sorry about that!

In the meantime, I think the easiest commands will probably be 'git
describe', 'git diff-files', and 'git diff-index' - feel free to switch to
one of those if you'd like. Of course, you're welcome to continue working on
'check-attr' - it'll be a little more complicated, but I don't think it'll
be as bad as, say, 'git mv' was (which needed substantial changes to its
sparse-checkout behavior before even starting the sparse index integration).

[1] https://git.github.io/SoC-2023-Ideas/

> 
> Thanks!
> 
> Shuqi Liang (1):
>   test check-attr pathspec inside sparse definition
> 
>  builtin/check-attr.c                     |  3 +++
>  t/t1092-sparse-checkout-compatibility.sh | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> 
> base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1