On Tue, 27 Oct 2020 09:38:41 GMT, Aleksey Shipilev <[email protected]> wrote:
>> Do you mean that you consider #505 and #548 dependent on this bug? If so, I >> think you misunderstand the purpose of the Github submit testing hook. You >> can -- and should! -- still do proper testing just as you did before the >> migration to Github. The Github action hook is a convenience. >> >> Unless there is a breakage or a regression, I don't think we should rush >> patches in. It's better to get things right. You say "then do everything >> else". Are there more functionality regarding x86 that you would like to see >> integrated in the Github submit hook? If so, I think it's better that you >> present it now, than just a piece at a time. >> >>> I believe the choice between "making the test code sparkling clean in one >>> go" and "making the product code sparking clean in one go" is obvious. >> >> I don't even understand what you mean by this. >> >>> These changes are logically independent. >> >> No, they are not. For one thing, this patch would explicitly introduce the >> "x32" naming, something JDK-8255305 is set. to remove. And this is just a >> trivial indication that both issues revolves around giving proper support >> for x86 in the submit flow. >> >> Just to be clear: I do not object to making x86 a first-class citizen in the >> submit hook! What I *do* object to is this piece-meal integration of two, or >> worse, three, PRs for what should have been one comprehensive PR. > >> Do you mean that you consider #505 and #548 dependent on this bug? If so, I >> think you misunderstand the purpose of the Github submit testing hook. You >> can -- and should! -- still do proper testing just as you did before the >> migration to Github. The Github action hook is a convenience. > > Oh yes, I must be misunderstanding this. Because I thought pre-submit tests > were the replacement for jdk-submit functionality, which was the Oracle's > requirement for push to shared code. Good to know it had been demoted to mere > suggestion. > >> Unless there is a breakage or a regression, I don't think we should rush >> patches in. It's better to get things right. You say "then do everything >> else". Are there more functionality regarding x86 that you would like to see >> integrated in the Github submit hook? If so, I think it's better that you >> present it now, than just a piece at a time. > > No, I mean to stop at `tier1` for x86_32. "Do everything else" means doing > the cosmetics like in this PR that does not affect the actual testing. > >> > I believe the choice between "making the test code sparkling clean in one >> > go" and "making the product code sparking clean in one go" is obvious. >> >> I don't even understand what you mean by this. > > I mean that bikeshedding the testing code means holding off the testing of > the product code. Making sure that product code is correct on x86_32 is more > important than making sure the testing code is cosmetically clean. > >> > These changes are logically independent. >> >> No, they are not. For one thing, this patch would explicitly introduce the >> "x32" naming, something JDK-8255305 is set. to remove. And this is just a >> trivial indication that both issues revolves around giving proper support >> for x86 in the submit flow. > > JDK-8255305 (#830) is set to add another instance of "x32", to already > existing "x32" in the build steps. That means #830 yields a logically > consistent `submit.yml`: it builds as "x32", it tests as "x32". This patch is > set to rename "x32" -> "x86" (*if*, *IF* we agree that rename even makes > sense!), yielding another logically consistent `submit.yml`. In this sense, > they are logically independent. > >> Just to be clear: I do not object to making x86 a first-class citizen in the >> submit hook! What I _do_ object to is this piece-meal integration of two, or >> worse, three, PRs for what should have been one comprehensive PR. > > There are "Request Changes" in #830 that you can use as Reviewer. I would > then oblige, but also reference this discussion. Because the next thing I > expect is someone else to come along and ask why "x32" -> "x86" rename is not > done separately from the addition of the actual testing. I would make that > comment myself for someone else's PR, because I prefer changes to do one > thing at a time. @shipilev My suggestion is that you take the commits from this PR and fold them into #830, and update the code added in #830 to match the naming. If you do that, I'll accept your patch. ------------- PR: https://git.openjdk.java.net/jdk/pull/869
