Date: Mon, 30 Jan 2017 07:28:08 -0600 From: John Marino <cbssports@marino.st> To: Torsten Zuehlsdorff <freebsd@toco-domains.de>, 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: <5725262c-4152-c711-e53b-a509742bcba1@marino.st> In-Reply-To: <4dd6d654-4525-c440-83b1-6a22215f6020@toco-domains.de> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > >> 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. > 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. 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. John --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5725262c-4152-c711-e53b-a509742bcba1>