Elia Pinto <gitter.spi...@gmail.com> writes: > 2015-12-11 15:40 GMT+01:00 Matthieu Moy <matthieu....@grenoble-inp.fr>: >> Elia Pinto <gitter.spi...@gmail.com> writes: >> >>> This is the second version of this patch. >>> Added the corrections suggested by Matthieu Moy ($gmane/282221) >> >> Sorry, but my main concern was that the patch could not be reviewed in >> good conditions as-is, and I think it still cannot be. It's very hard to >> spot which .PHONY rules you're adding and which are just code movement. >> You should really split this into one "code movement" patch and one >> "actual bugfix" patch. Or someone with better eyes than me should review >> the patch ;-). > > Ok. No problem. I thought there was no need for a patch so simple. But ok.
The point is: once a tricky bug was found in a patch (and I did on v1), you cannot claim anymore that it is "so simple". If it was that simple, you would have cought it before sending. >>> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a >>> test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS) >>> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter >>> %.o,$^) $(filter %.a,$^) $(LIBS) >>> >>> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1 >>> check-sha1:: test-sha1$X >>> ./test-sha1.sh >>> >>> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE >>> $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \ >>> $(SPARSE_FLAGS) $< >>> >>> -.PHONY: sparse $(SP_OBJ) >>> sparse: $(SP_OBJ) >> >> This "sparse" movement looks again contradictory with the goal announced >> in the commit message. > The idea was to group all the phony before all the target, not to put > the phony necessarily before the closest target. but ok I personally prefer the old way. I have no strong objection to changing, but currently your commit message says "Also put the .PHONY declaration immediately before the target declaration", which is clearly not a justification to do this change. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
