On Fri, 20 Nov 2020 14:24:12 GMT, Robin Westberg <rwestb...@openjdk.org> wrote:

> Currently Linux x64 testing in GitHub Actions depends on a few non-relevant 
> hotspot build-only jobs (such as zero) that prevents testing from being run 
> if those build were to fail. As the tests only require the x64 release and 
> debug builds to run and provide interesting feedback, we should separate 
> these other builds into a different job.
> 
> To reduce duplicated steps, all the existing hotspot-only builds have been 
> consolidated into a single job.

I like the cleanup! 

I do wonder though about phasing, probably unrelated to this patch. It seems to 
me that if some builds fail, then it is highly likely the contributor would 
push another update. So letting the tests run before all builds complete might 
be a waste of time on that failure path. It is probably a rare occasion, as 
normally everything is green, and thus unblocking the tests when base Linux x64 
builds are done is a good optimization.

.github/workflows/submit.yml line 380:

> 378:         continue-on-error: true
> 379: 
> 380:   linux_x64_hotspot_only:

This is a weird name for a job that also cross-compiles non-x64 versions. Maybe 
`linux_aux_builds` or some such?

.github/workflows/submit.yml line 469:

> 467:           name: transient_jdk-linux-x64_${{ 
> needs.prerequisites.outputs.bundle_id }}
> 468:           path: ~/jdk-linux-x64
> 469:         if: steps.build_restore.outcome == 'failure'

I may be blind, but I don't see any step with id `build_restore` in new file...

.github/workflows/submit.yml line 506:

> 504:         run: >
> 505:           sudo qemu-debootstrap
> 506:           --arch=${{ matrix.debian-arch}}

Style: `${{ matrix.debian-arch }}` (space before closing braces) -- also in 
other places.

.github/workflows/submit.yml line 532:

> 530:         run: >
> 531:           bash configure
> 532:           --with-conf-name=linux-x64-hotspot

That does not look a proper config name for cross-compiled jobs?

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

Changes requested by shade (Reviewer).

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

Reply via email to