chore: replace repository name and owner with Github context variables #124
Conversation
There was a problem hiding this comment.
Hi @matthiashermsen 👋
Thanks for the PR.
I've taken a look at it, but TBH I'm not convinced yet. In the end, it makes it harder to read which repository gets targeted, since now you don't have the single possible value hard-coded, but need to defer it from three different variables.
Am I missing a benefit here?
| @@ -76,7 +76,7 @@ jobs: | |||
| if: ${{ steps.get_next_version.outputs.hasNextVersion == 'true' }} | |||
| uses: docker/build-push-action@v5 | |||
| env: | |||
| DOCKER_REPOSITORY: ghcr.io/thenativeweb/get-next-version | |||
| DOCKER_REPOSITORY: ghcr.io/${{ github.repository }} | |||
There was a problem hiding this comment.
Why aren't you using the repository_owner and the event.repository.name here as well (or, vice-versa, why aren't you using this field above)?
Shouldn't both places use the same value, given that both have the same hard-coded value?
There was a problem hiding this comment.
There is no special reason, I just replaced the "hard-coded" parts with Github variables.
github.event.repository.namereturns the repository name ( without owner )github.repository_ownerreturns the repository ownergithub.repositoryreturns both with a slash
So you could use
${{ github.repository_owner }}/${{ github.event.repository.name }}
too, yeah :)
Hi @goloroden , |
|
Sorry for getting back to you that late … I have thought about this one, and TBH I don't see the benefit here, especially compared to that it makes it harder to guess what values are actually being used. This makes debugging more work, for something that happens once in a lifetime. Hence I'm going to close this one (without merging this). I hope you aren't mad at me for this. |
Sure, no problem :) |
No description provided.