| Message ID | 20181030220817.61691-1-sbeller@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Bring more repository handles into our code base] | expand |
Stefan Beller <sbeller@google.com> writes: > I also picked up the patch for pending semantic patches, as the > first patch, can I have your sign off, please? I find this step quite lacking. What was posted would have been perfectly fine as a "how about doing it this way" weatherbaloon patch, but as a part of real series, it needs to explain to the developers what the distinctions between two classes are, and who is to use the cocci patch at what point in the development cycle, etc., in an accompanying documentation update. So can we have both sign-off and write-up (perhaps cut&paste from the original e-mail discussion)? > Stefan Beller (23): > sha1_file: allow read_object to read objects in arbitrary repositories > packfile: allow has_packed_and_bad to handle arbitrary repositories > object-store: allow read_object_file_extended to read from arbitrary > repositories > object-store: prepare read_object_file to deal with arbitrary > repositories > object-store: prepare has_{sha1, object}_file[_with_flags] to handle > arbitrary repositories > object: parse_object to honor its repository argument > commit: allow parse_commit* to handle arbitrary repositories > commit-reach.c: allow paint_down_to_common to handle arbitrary > repositories > commit-reach.c: allow merge_bases_many to handle arbitrary > repositories > commit-reach.c: allow remove_redundant to handle arbitrary > repositories > commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary > repositories > commit-reach: prepare get_merge_bases to handle arbitrary repositories > commit-reach: prepare in_merge_bases[_many] to handle arbitrary > repositories > commit: prepare get_commit_buffer to handle arbitrary repositories > commit: prepare repo_unuse_commit_buffer to handle arbitrary > repositories > commit: prepare logmsg_reencode to handle arbitrary repositories > pretty: prepare format_commit_message to handle arbitrary repositories > submodule: use submodule repos for object lookup > submodule: don't add submodule as odb for push > commit-graph: convert remaining function to handle arbitrary > repositories > commit: make free_commit_buffer and release_commit_memory repository > agnostic > path.h: make REPO_GIT_PATH_FUNC repository agnostic > t/helper/test-repository: celebrate independence from the_repository It seems that this topic is full of commits with overly long title. > > Makefile | 6 +- > builtin/fsck.c | 3 +- > builtin/log.c | 6 +- > builtin/rev-list.c | 3 +- > cache.h | 2 + > commit-graph.c | 40 +++-- > commit-reach.c | 73 +++++---- > commit-reach.h | 38 +++-- > commit.c | 41 ++--- > commit.h | 43 +++++- > .../coccinelle/the_repository.pending.cocci | 144 ++++++++++++++++++ > object-store.h | 35 ++++- > object.c | 8 +- > packfile.c | 5 +- > packfile.h | 2 +- > path.h | 2 +- > pretty.c | 28 ++-- > pretty.h | 7 +- > sha1-file.c | 34 +++-- > streaming.c | 2 +- > submodule.c | 79 +++++++--- > t/helper/test-repository.c | 10 ++ > 22 files changed, 459 insertions(+), 152 deletions(-) > create mode 100644 contrib/coccinelle/the_repository.pending.cocci > > git range-diff origin/sb/more-repo-in-api... > [...] # rebased to origin/master I offhand do not recall what happened during these 100+ pacthes. DId we acquire something significantly new that would have conflicted with this new round of the topic? I do not mind at all seeing that a series gets adjusted to the updated codebase, but I do dislike seeing it done without explanation---an unexplained rebase to a new base is a wasted opportunity to learn what areas of the codebase are so hot that multiple and separate topics would want to touch them. > -: ---------- > 116: 4ede3d42df Seventh batch for 2.20 > -: ---------- > 117: b1de196491 Makefile: add pending semantic patches > 1: 1b9b5c695e = 118: f1be5eb487 sha1_file: allow read_object to read objects in arbitrary repositories > 2: 33b94066f2 = 119: 9100a5705d packfile: allow has_packed_and_bad to handle arbitrary repositories > 3: 5217b6b1e1 = 120: a4ad7791da object-store: allow read_object_file_extended to read from arbitrary repositories > 4: 2b7239b55b ! 121: 5d7b3a6dd9 object-store: prepare read_object_file to deal with arbitrary repositories > @@ -19,10 +19,10 @@ > Signed-off-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > > - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci > + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci > new file mode 100644 > --- /dev/null > - +++ b/contrib/coccinelle/the_repository.cocci > + +++ b/contrib/coccinelle/the_repository.pending.cocci > @@ > +// This file is used for the ongoing refactoring of > +// bringing the index or repository struct in all of > @@ -36,6 +36,7 @@ > +- read_object_file( > ++ repo_read_object_file(the_repository, > + E, F, G) > ++ Is it necessary for this file to end with a trailing blank line? Thanks.
On Tue, Oct 30, 2018 at 11:41 PM Junio C Hamano <gitster@pobox.com> wrote: > > Stefan Beller <sbeller@google.com> writes: > > > I also picked up the patch for pending semantic patches, as the > > first patch, can I have your sign off, please? > > I find this step quite lacking. > > What was posted would have been perfectly fine as a "how about doing > it this way" weatherbaloon patch, but as a part of real series, it > needs to explain to the developers what the distinctions between two > classes are, and who is to use the cocci patch at what point in the > development cycle, etc., in an accompanying documentation update. if only we had documentation [as found via "git grep coccicheck"] that I could update ... I'd totally do that. I guess this asks for documentation to begin with, now? > So can we have both sign-off and write-up (perhaps cut&paste from > the original e-mail discussion)? I'll see where to put the docs; I assumed commit messages are enough. 63f0a758a0 (add coccicheck make target, 2016-09-15) is what I found nice. > > t/helper/test-repository: celebrate independence from the_repository > > It seems that this topic is full of commits with overly long title. yep. > > git range-diff origin/sb/more-repo-in-api... > > [...] # rebased to origin/master > > I offhand do not recall what happened during these 100+ pacthes. > DId we acquire something significantly new that would have > conflicted with this new round of the topic? I do not mind at all > seeing that a series gets adjusted to the updated codebase, but I do > dislike seeing it done without explanation---an unexplained rebase > to a new base is a wasted opportunity to learn what areas of the > codebase are so hot that multiple and separate topics would want to > touch them. From the point of view of these large scale refactorings, all of the code is hot, e.g. the patch that was present in the RFC "apply all semantic patches" would clash with nearly any topic. As I do not carry that patch any more, I do not recall any conflicts that needed to be resolved. Thanks.
Stefan Beller <sbeller@google.com> writes: >> What was posted would have been perfectly fine as a "how about doing >> it this way" weatherbaloon patch, but as a part of real series, it >> needs to explain to the developers what the distinctions between two >> classes are, and who is to use the cocci patch at what point in the >> development cycle, etc., in an accompanying documentation update. > > if only we had documentation [as found via "git grep coccicheck"] > that I could update ... I'd totally do that. > I guess this asks for documentation to begin with, now? So far, we didn't even need any, as there was no "workflow" to speak of. It's just "any random developer finds a suggested update, either by running 'make coccicheck' oneself or by peeking Travis log, and sends in a patch". But the "pending" thing has a lot more workflow elements, who is responsible for noticing update suggested by "pending" ones, for updating the code, and for promoting "pending" to the normal. These are all new, and these are all needed as ongoing basis to help developers---I'd say the way Documentation/SubmittingPatches helps developers is very close to the way this new document would help them.