Test only supported Groovy versions and automate their testing

[keeganwitt]

No description provided.

[bmarwell]

Not sure you saw it, I was working on the same thing: #268

[bmarwell]

Why would you do that?

[keeganwitt]

Because what's often call the "strict" mode of Bash isn't on by default.

[bmarwell]

Yeah but the bash is terminated right afterwards. It does not affect subsequent runs.

[keeganwitt]

For errexit, It's true it won't affect other runs or jobs, but it would affect subsequent commands in the same job.
At the moment, we only have a single command, so it doesn't add any value, but if there were ever a second command added, the second command would execute even if the first command failed. That's usually not desirable.

nounset was there to prevent us from accidentally using a variable that wasn't set.

pipefail is another one that doesn't matter right now because we've only got 1 command taking from a pipe (and it's one I added with this PR).

I just had added this since it's what I put on all my Bash scripts (unless I have a specific reason not to).

[bmarwell]

This is the default

[keeganwitt]

Oh, I must have mis-read. I thought it was Zulu. I don't have a strong preference here.

[bmarwell]

No I meant arch: x64 is the default

[bmarwell]

No I meant arch: x64 is the default

[keeganwitt]

Ah, gotcha. I had that from back in the day for whatever reason. (possibly no reason).

[bmarwell]

'|' should not be used here, better use variables in github

[keeganwitt]

I'm confused how I could use variables in this case. The values are different, depending on which version of the job from the matrix is running. Ideally, I could include this along with the version in the matrix, but I didn't see a way to do that.

[bmarwell]

I did it like that:

https://github.com/groovy/GMavenPlus/pull/268/files#diff-944291df2c9c06359d37cc8833d182d705c9e8c3108e7cfe132d61a06e9133ddR46-R50

        run: >-
          ./mvnw ${MAVEN_ARGS}
          ${{ matrix.groovy-version != '' && format('{0}{1}', '-DgroovyVersion=', matrix.groovy-version) || '' }}
          ${{ startsWith( matrix.groovy-version , '3' ) && '-DgroovyGroupId=org.codehaus.groovy' || '' }}
          clean install invoker:install invoker:run

There are other ways, too. But on Github, the shell could change (e.g. ksh on AIX, zsh on Mac). So I would not want to rely on the shell.

[bmarwell]

So while you started working on your own PR AFTER I started the other one, you incorporated the changes from #266 which makes this PR way to big. Is this intended?

[keeganwitt]

So while you started working on your own PR AFTER I started the other one, you incorporated the changes from #266 which makes this PR way to big. Is this intended?

I didn't see you'd already opened a PR. But I only opened this PR to experiment with what this might look like. That's why it's a draft. We'll probably merge other smaller PRs, and not actually this one.

[bmarwell]

Okay. But on the bright side, we now have two different PRs so we can pick the things we like best from both.