drone-plugins / drone-codecov

Drone plugin to publish coverage to Codecov
http://plugins.drone.io/drone-plugins/drone-codecov
Apache License 2.0
4 stars 4 forks source link

Include OS in tag suffix #7

Closed bradrydzewski closed 6 years ago

bradrydzewski commented 6 years ago

I recommend including the operating system in the suffix so that we don't run into naming issues when we support windows. For example plugins/codecov:latest-window-amd64 and plugins/codecov:latest-linux-amd64

  publish_linux_amd64:
    image: plugins/docker:17.05
    pull: true
    secrets: [ docker_username, docker_password ]
    group: docker
    repo: plugins/codecov
    auto_tag: true
-   auto_tag_suffix: amd64
+   auto_tag_suffix: linux-amd64
    dockerfile: Dockerfile
    when:
      event: [ push, tag ]
tboerger commented 6 years ago

Do you think that we need multiple architectures for windows? AFAIK only amd64 is supported. So in that case the current tags should work (with the custom manifest file part of the repo)

bradrydzewski commented 6 years ago

It is more about consistency with the Drone core project [1] and other projects such as Portainer that following similar conventions [2]. If we publish linux-amd64 and windows-amd64 we should following the same convention for arm and arm64 even if no other operating systems are supported in the short term :)

[1] https://hub.docker.com/r/drone/agent/tags/ [2] https://hub.docker.com/r/portainer/portainer/tags/

tboerger commented 6 years ago

Ok, when I introduced that for this plugin I took the drone server tagging. I'm fine with changing that, in that case we can also drop the manifest config for the manifest step because it can get mapped properly.

tboerger commented 6 years ago

As you can see I have create a PR now to change the current behavior.