Date: Mon, 30 Jan 2017 08:02:51 -0600 From: John Marino <freebsd.contact@marino.st> To: Torsten Zuehlsdorff <freebsd@toco-domains.de>, 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: <5eac2fff-ed34-7e54-574e-1134b290a736@marino.st> In-Reply-To: <23d56a5a-f7a0-5d03-cbef-c08bdafa417b@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> <5725262c-4152-c711-e53b-a509742bcba1@marino.st> <23d56a5a-f7a0-5d03-cbef-c08bdafa417b@toco-domains.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/30/2017 07:50, Torsten Zuehlsdorff wrote: > On 30.01.2017 14:28, John Marino wrote: >> >> 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. nor checked commit log for php71 (or freshports). svn diff wouldn't have helped here because the files were added, not repocopied. > >>>> 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. And this is basically a separate and irrelevant topic. After a two month period, the review could have been updated to capture commits to the original port since the original review as well. > >>> 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... I don't think it's that hard. Freshports shows commits in a quick fashion. So yes, I'm saying if a review is old, new changes should be tracked. I think that's expected. I do appreciate that it was a very big patch set and this is all easier said than done. I was focusing on the main port rather than the extensions. water under the bridge. basically its a learning opportunity now. - review time - svnrepo - tracking original - diff Bottom line: You did a good jobs with php71 and there are some minor things you could have done better. Those things weren't unavoidable IMO. 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?5eac2fff-ed34-7e54-574e-1134b290a736>