RFR: 8255412: "Linux x32" should be "Linux x86" in submit workflow

[Aleksey Shipilev]

On Tue, 27 Oct 2020 09:38:41 GMT, Aleksey Shipilev <shade at openjdk.org> 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.

Thanks. Again: please record this suggestion as formal "request changes" in #830, so that #830 PR history is consistent and clear to outside observer without reading the whole thread.

-------------

PR: https://git.openjdk.java.net/jdk/pull/869