On Mon, 19 Aug 2024 13:26:04 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> Yes, this is somewhat more akin to what I was thinking. But I was actually > thinking about a more encompassing rewrite of that method. > > First of all, GHA variables did not exist when that code were written, so > they had to be secrets, even though it does not make sense in this context. > These control variables should be changed from secrets to variables, or the > code rewritten so it can support both. > > Secondly, I think there is a use-case for having both a variable which just > appends to the default set of platforms/tests, and one that replaces the > default value. > > Third, there has been requests to change what tests are run (e.g. tier-2) as > well as which platforms are tested. I think it would be good to let such > functionality tie in with any other changes in this area. > > But, just to be clear, I don't think it is necessary, nor even appropriate, > for you to address any of these changes in this PR. In fact, I would have > been happy with the previous iteration as well. But this version is okay as > well, if you can move the exclude list to a more suited location. yeah I totally agree with this idea, I'm happy to make these changes but that's outside of the scope of this PR, I'll test some changes in my local environment. > .github/workflows/main.yml line 84: > >> 82: >> 83: # List of platforms to exclude by default >> 84: EXCLUDED_PLATFORMS=("alpine-linux-x64") > > I'm not to fond of declaring a list like this in the middle of a code body. > Perhaps you can set it up as a non-required workflow input with alpine as the > default value, declared next to the default list of platforms? yeah this makes sense, I'll make that change now ------------- PR Comment: https://git.openjdk.org/jdk/pull/20577#issuecomment-2296595922 PR Review Comment: https://git.openjdk.org/jdk/pull/20577#discussion_r1721799725