On Thu, Aug 25, 2022 at 8:54 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > On Thu, Aug 25, 2022 at 9:25 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > On Tue, Aug 16, 2022 at 8:50 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > > > > Failed or incomplete test runs can leave temporary test files in > > > the binder test directory, remove them with 'make clean'. > > > > > > mkfifo: cannot create fifo \ > > > 'binder/manager_flag': File exists > > > mkfifo: cannot create fifo \ > > > 'binder/service_provider_flag': File exists > > > > > > Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx> > > > --- > > > tests/binder/Makefile | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/binder/Makefile b/tests/binder/Makefile > > > index e78ad16..b89d4db 100644 > > > --- a/tests/binder/Makefile > > > +++ b/tests/binder/Makefile > > > @@ -18,6 +18,6 @@ endif > > > all: $(TARGETS) > > > > > > clean: > > > - rm -f $(TARGETS) > > > + rm -f $(TARGETS) manager_flag service_provider_flag > > > > > > $(TARGETS): $(DEPS) > > > > Thanks, though it would be good to do the same cleanup also in other > > tests. > > I agree, but I didn't/don't have the time to do that for the other > tests, I saw this one and I fixed it :) > > > I extended your patch to almost all other dirs in [1] - is it > > OK if I apply that version? > > No, but not because I think those changes are wrong, it's because I > don't agree with the approach. Let me try to explain ... > > It is my personal opinion that with few exceptions, a maintainer > should not alter a patch significantly beyond the normal fuzz that can > sometimes happen when there are merge conflicts. Of course there are > trivial changes sometimes, e.g. a missing semicolon, whitespace > issues, etc. which are okay to fixup (with a note in the commit!), but > changes of more than a couple of lines, or changes that impact the > logic of the patch, are not something a maintainer should be doing as > a normal practice. I am not a lawyer, so please don't take this as a > valid interpretation of the laws involved, but I can see legal reasons > for this: if the maintainer changes the patch in a significant way, I > imagine that could potentially muddy the idea of authorship, would the > maintainer now also be considered an author of that patch? How could > one clearly distinguish between the original author's code and the > mainatiner's changes? Sure, there is the mailing list, but what if > the mailing list is not available and all you have is the git log? > However, the biggest issue that I see is that of community building. > Having a back-and-forth with a patch contributor can help both welcome > them to the community and teach them what is expected from a patch > submission point of view. It might be easier and quicker for you just > to edit the existing patch, but it's better for the community to take > the time and ask the original submitter if they could make the change. > > Does that make sense? Yes, I agree. Looking back at what I did with the patch and communication I can see I went the completely wrong way about it and I apologize. Actually there is really no good reason for me not to simply add the extra changes as a separate patch/commit... I guess I just instinctively tried to keep the one logical change in one patch and didn't think it through properly... I'll scratch that embarrassing commit and just post the extra changes as another patch. -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
