Date: Mon, 30 Jan 2017 14:50:46 +0100 From: Torsten Zuehlsdorff <freebsd@toco-domains.de> To: John Marino <cbssports@marino.st>, marino@freebsd.org, ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org Cc: Mathieu Arnold <mat@FreeBSD.org> Subject: Re: svn commit: r432561 - head/lang/php71 Message-ID: <23d56a5a-f7a0-5d03-cbef-c08bdafa417b@toco-domains.de> In-Reply-To: <5725262c-4152-c711-e53b-a509742bcba1@marino.st> References: <201701271852.v0RIqOvW033749@repo.freebsd.org> <6aebcd6a-0439-f23a-be9d-e06f46d3511b@toco-domains.de> <ce44c753-2d68-0899-b07c-cfaa92de1158@marino.st> <4dd6d654-4525-c440-83b1-6a22215f6020@toco-domains.de> <5725262c-4152-c711-e53b-a509742bcba1@marino.st>
next in thread | previous in thread | raw e-mail | index | archive | help
On 30.01.2017 14:28, John Marino wrote: > On 1/30/2017 05:15, Torsten Zuehlsdorff wrote: >> >> >> On 29.01.2017 19:55, John Marino wrote: >>>> On 27.01.2017 19:52, John Marino wrote: >>>>> Author: marino >>>>> Date: Fri Jan 27 18:52:24 2017 >>>>> New Revision: 432561 >>>>> URL: https://svnweb.freebsd.org/changeset/ports/432561 >>>>> >>>>> Log: >>>>> lang/php71: Bring DTRACE exclude for DF from lang/php70 >>>>> >>>>> Maybe this would have been caught with SVN copy? Oh well. >>>>> >>>>> Approved by: just-fix-it >>>> >>>> Its funny how "just-fix-it" often translate in "just-broke-it" - see >>>> r432567. >>>> >>>> But beside that back to being serious: why did nobody suggest a fix? If >>>> repo-copy is this needed, we can do it right now? >>>> svn rm lang/php71 && svn cp -r432321 lang/php70 lang/php71 && some >>>> files >>>> copied would be all. >>>> I took mat into CC because i want to know if something speaks >>>> against it >>>> or if just nobody thought about it. >>>> >>>> Also fun note: with an svn copy the DTRACE option wouldn't be included >>>> too. I finished the patch 2 month ago, long before DTRACE was >>>> introduced. So it wouldn't be there. >>> >>> Which would be an error of process. >>> - svn cp php70 >>> - overwrite with your working copy >>> - view with svn diff. The new additions in the last 2 months would be >>> apparent. Thus the 3rd step wasn't done. >> >> I did exactly this. 2 months ago there was no DTRACE in php70. So >> copying the working-copy of php71 over the repo-copy would have not >> included the DTRACE option. Therefore the repo-copy wouldn't have had >> helped. > > EXACTLY. > Your mistake is leaving this at two months ago. > I meant check differences the day of committing, not check differences 2 > months ago. If you had not assumed that zero changes happened to php70 > that would affect php71 in a two month period, then you would rechecked > the diff before you actually commit and seen them. I would have noticed > the changes because I always run "svn diff" before committing. I do a svn diff before every commit. Sadly i did not see this mistake within this some hundred lines. >>> You misunderstood r432567. >>> That wasn't to correct my commit. >>> That was to add the second missing item, DTRACE for aarch. >>> >>>> I just noted in this 2 month how hard it is to find somebody for review >>>> and second how hard it is to get such a patch reviewed. First the >>>> review.freebsd.org didn't work for me because it contains a bug with >>>> umlaut for years. And second a repocopy could not be displayed. >>> >>> Yes, the diff would have been displayed. >>> That's the benefit of repocopy. >> >> Maybe i wasn't clear enough: the repo-copy could not displayed in >> review.freebsd.org. If you say: its possible. Okay, than its just the >> umlaut-bug preventing the display of my patch. > > I am not talking about review.freebsd.org. > I'm talking about svn. Read my statement again. I explicitly talk about reviews.freebsd.org and also about its incapability of displaying a repo-copy. >> But - no, you are not right this time. Doing a repo-copy without >> changing the copy will cause a svn diff to just list the files. The diff >> didn't even contain the revision copied from. To bring a path like this >> into the review tool you need at least the --show-copies-as-adds param, >> which defeats the purpose of a repo-copy in review. > > I don't think anybody was talking about review. I was - i explicitly wrote it. > If you do repocopy correctly, we all get a commit mail that shows the > differences between the new version and the version it was copied from. > In this case, we got what looked like complete files added which is how > mat know that svncopy wasn't used. I know. You missed my point: making an error with repo-copy is hard to catch - even with the help of others. reviews.freebsd.org is not able to display a repo-copy. "svn diff" doesn't even show the revision of the copy used. So you need to do it timely (with is hard with such a big patch) or you need to track everything in the meantime to decide if its needed to do all the work again. And again. And again... Greetings, Torsten
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?23d56a5a-f7a0-5d03-cbef-c08bdafa417b>