On Tue, 27 Oct 2020 09:22:39 GMT, Magnus Ihse Bursie <[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. 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. ------------- PR: https://git.openjdk.java.net/jdk/pull/869
