athieriot / drone-artifactory

Drone plugin for publishing build artifacts to Artifactory
Apache License 2.0
10 stars 12 forks source link

Lack of DRONE_VERSION #21

Open k0nserv opened 6 years ago

k0nserv commented 6 years ago

So I think the DRONE_VERSION environment variable was removed from Drone at some point between 0.5 and 0.8. This causes the plugin to go down the pre 0.5 code path and fail despite the fixes in #19 and #20. I've asked in the issue that requested the variable be introduced. https://github.com/drone/drone/issues/1415

athieriot commented 6 years ago

Oh man.

Maybe there is an another way to detect the version. Hopefully.

k0nserv commented 6 years ago

Yeah I'm not sure. Another option is to publish tagged versions for the plugin with different support and elliminate the 0.4 support. However there will be other users depending on latest being compatible with 0.4 so that's not a good idea as it stands

athieriot commented 6 years ago

Let's wait for their answers and I can push a 0.4 tag along.

I don't think it's too harsh to make latest going further and be, as its name suggest, the latest (Considering they are already version 0.8)

On 16 November 2017 at 16:53, Hugo Tunius notifications@github.com wrote:

Yeah I'm not sure. Another option is to publish tagged versions for the plugin with different support and elliminate the 0.4 support. However there will be other users depending on latest being compatible with 0.4 so that's not a good idea as it stands

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/athieriot/drone-artifactory/issues/21#issuecomment-344986428, or mute the thread https://github.com/notifications/unsubscribe-auth/AAoZjZmZ6Pn0PC2dhuIL4UElo69bUefTks5s3GiHgaJpZM4Qgyoh .

k0nserv commented 6 years ago

Well there aren't any tags so people didn't previously have the option to depend on a specific version. I know a lot of our usage would break with such a change

tboerger commented 6 years ago

Just to note, everything <= 0.5 is EOL on the drone core side.

Poogles commented 6 years ago

Can't we just look for version unique to later versions (or even prior) env variables? Looking at latest and 0.5 vs 0.4 then DRONE_PREV_COMMIT_SHA might be an option?

k0nserv commented 6 years ago

It turns out you can do this

    environment:
      - DRONE_VERSION=0.8 # Attempt to fool the artifactory plugin
athieriot commented 6 years ago

Do you like that or... ?

I still think we could benefit from having Docker tags anyway

k0nserv commented 6 years ago

It works on latest to make drone-artifactory work with Drone 0.8. We could definitely benefit from tags, however making breaking changes to latest would break a lot of peoples deployments. I know that internally we have a lot of projects that use 0.4 with drone-artifactory:latest. These would all break if changes that aren't compatible with Drone 0.4 were pushed to latest

athieriot commented 6 years ago

I think I feel a bit weird of using an env variable (I mean, that the user has to set up) in order to toggle Drone implementation.

As a user yourself, do you see that as an issue?

k0nserv commented 6 years ago

It's definitely a workaround, however maintaining Drone 0.4 compatibility is important.

athieriot commented 6 years ago

Maybe we could keep latest as having 0.4 support and new tags for the following?

On 30 November 2017 at 14:00, Hugo Tunius notifications@github.com wrote:

It's definitely a workaround, however maintaining Drone 0.4 compatibility is important.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/athieriot/drone-artifactory/issues/21#issuecomment-348195607, or mute the thread https://github.com/notifications/unsubscribe-auth/AAoZjVWK0DNgBuRtajX8-vv4Li-Yi6yvks5s7rTigaJpZM4Qgyoh .

tboerger commented 6 years ago

Keeping a dead version is important?

k0nserv commented 6 years ago

It wouldn't be if tagged versions had been published previously and their used encouraged. Given that that's not the case, yes it's important as it would break usage for anyone with < Drone 0.5

tboerger commented 6 years ago

I don't believe that there are lots of such old installations flying around but it's up to you.