{"thread":{"id":"55720","subject":"Re: [PATCH] Packfile-uris support excluding commit objects","startedAt":"2021-05-18T08:08:50Z","lastAt":"2021-05-18T08:08:50Z","messageCount":1,"participants":["Teng Long"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"424841","messageId":"CADMgQSReX+Ue0AtPh6JWNde-pyFC5-r+juQFoKd5fZp5Wv6_Ow@mail.gmail.com","threadId":"55720","inReplyTo":null,"subject":"Re: [PATCH] Packfile-uris support excluding commit objects","fromName":"Teng Long","fromEmail":"dyroneteng@gmail.com","sentAt":"2021-05-18T08:08:36Z","receivedAt":"2021-05-18T08:08:50Z","isPatch":true,"sender":{"key":"dyroneteng@gmail.com","avatar":"https://avatars.githubusercontent.com/u/7803958?v=4"},"body":"To Ævar Arnfjörð Bjarmaso wrote:\n\n> It seems like this and your\n> http://lore.kernel.org/git/20210506073354.27833-1-dyroneteng@gmail.com\n> should be part of one series, not split up.\n\nObviously, the current series of patches will be longer, and the patches\nof the separate documents you mentioned can be repaired and released in\nadvance. The latter, Junio said, will be added to the queue.\n\n> Per my understanding in\n> https://lore.kernel.org/git/87o8hk820f.fsf@evledraar.gmail.com/ this +\n> Jonathan's earlier bfc2a36ff2a (Doc: clarify contents of packfile sent\n> as URI, 2021-01-20) still makes this whole thing more confusing that it\n> needs to be.\n\n> I think we should just have a new uploadpack.excludeObject, and document\n> that uploadpack.blobpackfileuri is an (unfortunately named) synonym for\n> it. I.e. the actual implementation doesn't care about the objec type it\n> just excludes any object listed via an oidmap. No?\n\nRegarding the naming of \"uploadpack.blobpackfileuri\" and future\nscalability issues, I have similar feelings. In next round, I will follow\nyour naming suggestions about \"uploadpack.excludeObject\". In addition,\nthe naming modification may cause a compatible question, although I know\npackfile-uris is an experimental feature, I still hope to get some\ncompatibility-related suggestions.\n\n> I realize you're probably not a native English speaker (neither am I),\n> but I honestly can't understand that \"This work will be done in a\n> further patch recently.\". Do you mean something like:\n>        ......\n\nThanks for correcting, I may rewrite the commit message in the next\npatch. I'm not a native English speaker, improving :)\n\n> Please send the earlier doc cleanup + the spec change for this + any doc\n> updates as one series.\n\nThe reason for splitting the two patches is mentioned above, and the\ncorresponding document modification to support the exclusion of commit\nwill be added in the next patch series.\n\n> Nit: Split this across two lines.\n> Indending with spaces.\n> More indenting with spaces, also don't need the {} here.\n> Don't indent the \"then\", also spaces...\n> Use \">objh\" not \"> objh\".\n\nThanks, will correct them in the next round.\n\n> I think by having a uploadpack.excludeObject documented as the primary\n>interface to this we could just say \"object already listed by an earlier\n>exclusion\" or something like that.\n\nThanks, will refer to your suggestions.\n\n> This whole if/else seems like it could be better split up by discovering\n> the variable first, using that as a variable, and then avoiding the\n> duplication. But if we just used uploadpack.excludeObject...\n\nI will try to modify it, but I am not sure to fully understand what you\nmean. If this problem persists in the next patch, please help to point\nout the problem.\n\n> Put stuff like this in \"test_when_finished\"\n> You can just use test_commit here, no?\n\nThanks, now I know about \"test_when_finished\" and \"test_commit\".\nIn addition, there are some problems with using \"rm\" directly in t5702,\nI will replace them in the next patch series.\n\n> Personally I'd just skip this whole \"rev-parse HEAD\" etc. and just pass\n> the tag name(s) created by earlier test_commit, then have\n> configure_exclusion ust always do a rev-parse...\n\nThanks, will use tag name(s) instead of \"HEAD\".\n"}]}