Skip to content

Extend for multi projects#3

Open
lewis-king wants to merge 6 commits intomasterfrom
extend-for-multi-projects
Open

Extend for multi projects#3
lewis-king wants to merge 6 commits intomasterfrom
extend-for-multi-projects

Conversation

@lewis-king
Copy link

No description provided.

description: 'A GitHub auth token to be able to create the pull request'
required: true
gradle-path:
version-filepath:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave this as gradle-path, as it better describes what its relating to, the use of "version" is not particularly clear, also this could be a breaking change where as all the other changes are not breaking, and its specific to gradle, whereas we also deal with pom's in this script but we don't use the variable for poms

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. well; I did not find where we were using this gradle-path to begin with. So having it and using it instead of having it and not using it is 100% an improvement.

This being said, there is no good reason to:

  1. rename it to version-filepath (and lose the "coupling" to Gradle) - so far so good...
  2. add it to the Gradle part of the code - yep, great, thank you..
  3. not do it for the Maven part...

I mean supporting the Maven part of this is not the most fun, but it's out there, and if you go down the route of un-specialising those variable/property names, that's fine, but if you do, you really need to do the Maven part.

If not, then keep it gradle-path and let us be happy with the improvement on the Gradle part of the tool.

GIT_COMMITTER_NAME: ${{ steps.config.outputs.git-committer-username }}
GIT_COMMITTER_EMAIL: ${{ steps.config.outputs.git-committer-email }}
CURRENT_VERSION: ${{ steps.config.outputs.current-version }}
VERSION_FILEPATH: ${{ steps.config.outputs.version-filepath }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RELATIVE_GRADLE_FILEPATH would be better

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, assuming the change is not done on the Maven part of the tool. If it is, then the new name is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants