Skip to content

Composite Docker action #208

Open
@Brutus5000

Description

Hi guys,

this is not really an issue, but some feedback that you hopefully consider:
I'm aware I probably sound like a whiny crybaby, but please believe me that I admire your efforts... but seriously: with v2 you raped a good working Github action.

And yes yes, you're probably thinking: Why don't you fix it then yourself? But I'm already managing two dozen repositories for the FAForever project and I just need an easy and working CI infrastructure. [And v1 is still available, so technically no problem.]

Why do I use this action?
I prefer Github actions because they are easier to read and simplified building blocks over writing it in a shell script, even if they are a few (!) lines longer

And v1 of this action is the perfect solution for the workflow I need (with some Github if expression magic):

      - name: Build and push Docker images
        if: github.ref == 'refs/heads/develop' || startsWith(github.ref, 'refs/tags')
        uses: docker/[email protected]
        with:
          username: ${{ secrets.DOCKER_USERNAME }}
          password: ${{ secrets.DOCKER_PASSWORD }}
          repository: myproject/myrepo
          tag_with_ref: true

What has changed?
With v2 I need to split it off in 3 steps (+ an additional one considering #100):

  • one step declaring the tag I want to push
  • one step for a login (yes, if I want to push I ALWAYS want to login)
  • one step for the build & push

Eventually I end up with 3x the lines of code that I had before. No thanks, I rather stick with the previous version or go back to manual shell commands.

Here are some ideas to improve on v2:

  • I do understand that it makes sense to have a login action if you want to pull images from a protected repository. But why does this have to affect the build-and-push step? If already logged in for other reason, this is the special case not the default, so my proposal would be to either ignore this case (double login then) or to offer an login opt-out config flag instead.
  • I do understand that the tag_with_ref flag was quirky and limits some scenarios like pushing multiple tags and I actually like that there is another way now to configure this now. But was it necessary to drop the existing flag? Maybe you can consider bringing it back under a more meaningful name.
  • Looking at the upgrade guide it seems that context and file are mandatory now (didn't test it actually) but then again: why would I need to declare what is the 99% default case? So if it's not mandatory I'd to remove it from the upgrade document, and if it is mandatory I'd suggest to offer default settings for a classic Dockerfile build

I'd love to see a very powerful action as it is now, with sensible default settings preconfigured for the simple use cases.

Love you guys! Keep up the good work! ❤️

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions