Re: [PATCHv2 00/24] Bring more repository handles into our code base]
Stefan Beller 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.
Re: [PATCHv2 00/24] Bring more repository handles into our code base]
On Tue, Oct 30, 2018 at 11:41 PM Junio C Hamano wrote: > > Stefan Beller 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.
Re: [PATCHv2 00/24] Bring more repository handles into our code base]
Stefan Beller 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 objec
[PATCHv2 00/24] Bring more repository handles into our code base]
This resends sb/more-repo-in-api
On Thu, Oct 25, 2018 at 2:14 AM SZEDER Gábor wrote:
> On Tue, Oct 16, 2018 at 04:35:49PM -0700, Stefan Beller wrote:
> > This converts the 'show_submodule_header' function to use
> > the repository API properly, such that the submodule objects
> > are not added to the main object store.
>
> This patch breaks the test suite with 'GIT_TEST_COMMIT_GRAPH=1', in
> particular 't4041-diff-submodule-option.sh' fails with:
> [...]
I debugged into this and the last four patches will fix it.
I also picked up the patch for pending semantic patches, as the
first patch, can I have your sign off, please?
This also addresses Jonathans feedback in
https://public-inbox.org/git/20181019203750.110741-1-jonathanta...@google.com/
A range-diff is below.
Thanks,
Stefan
SZEDER Gábor (1):
Makefile: add pending semantic patches
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
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
-: -- > 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
Signed-off-by: Junio C Hamano
- 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/nu
