-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Update acceptance_test.yml to reflect new -cli.jar naming. #1168
fix: Update acceptance_test.yml to reflect new -cli.jar naming. #1168
Conversation
…ject. Currently, project version info is passed in via a `versionTag` env variable, which is only specified in the Github workflow environment. Consequently, `project.version` ends up being `null` when run locally. This causes weirdness for tools that expect a valid version number. This change switches the project to use the axion-release plugin, which determines the project version by looking at the git repo's tags. Along with that switch, I've moved the project group and version specification into the root build.gradle allprojects {} such that these values do not need to be specified in each sub-poject. I also did some related clean-up for our jitpack configuration (and Maven repository modules, should we every decide to publish them). I believe our current jitpack config doesn't actually work correctly because we only publish artifacts from the main package, but not core. You can see this in practice when looking at: https://jitpack.io/com/github/MobilityData/gtfs-validator/3.0.1/gtfs-validator-3.0.1.pom Notice how it has a dependency on the `core` module but we don't actually publish it anywhere. If you actually attempt to use our project as a jitpack dependency, it will fail. The change was to also publish the core module and to specify more natural artifact names.
longer needed now that project version is determined within Gradle directly. Also fixup references to shadow jars, since they no longer include the versionTag in the filename (also _cli => -cli).
We can't update this code until the PR actually goes in on the master branch.
I also cleaned up some other variables from the docker config that don't seem to be used.
…previous commit.
…now that PR MobilityData#1147 has been committed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bdferris-v2!
Oh, just realized this is still marked as "draft". Was it ready for review? |
@barbeau I was just waiting to see if the Acceptance Test actually ran successfully before flipping over for review. It looks like it's working! @maximearmstrong or @isabelle-dr any chance you could review? I think Acceptance Test will continue to fail until we get this in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you @bdferris-v2
Summary:
As part of issue #1139, PR #1147 changed how shadow jar artifacts are named. We updated that naming in all GitHub workflows except for one place in
acceptance_test.yml
where it continued to reference the jar produced on themaster
branch with the old naming scheme. Now that PR #1147 is in on themaster
branch, we can resolve the TODO there to reference the new jar naming scheme.Closes #1139.
gradle test
to make sure you didn't break anything