Downgrade github.com/urfave/cli to v1 #1286
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It's possible to upgrade to the latest v1 release, which uses go modules. I've held off on that fro now though, and stuck with a pure rollback. Here's what that upgrade would look like. $ go get github.com/urfave/cli
go: github.com/urfave/cli upgrade => v1.22.4
[jh@tak agent (rollback-ufave-cli-upgrade)]$ git diff
diff --git a/go.mod b/go.mod
index ff1dfbb3..6af3a340 100644
--- a/go.mod
+++ b/go.mod
@@ -26,7 +26,7 @@ require (
github.com/qri-io/jsonschema v0.0.0-20180607150648-d0d3b10ec792
github.com/sergi/go-diff v1.0.0 // indirect
github.com/stretchr/testify v1.5.1
- github.com/urfave/cli v0.0.0-20180226030253-8e01ec4cd3e2
+ github.com/urfave/cli v1.22.4
golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073
golang.org/x/oauth2 v0.0.0-20181003184128-c57b0facaced
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f // indirect |
yob
force-pushed
the
rollback-ufave-cli-upgrade
branch
from
c906142 to
264e721
Compare
yob
force-pushed
the
rollback-ufave-cli-upgrade
branch
from
264e721 to
3e33d6a
Compare
In #1233 we upgraded github.com/urfave/cli from v1 to v2. There was no functional need for the upgrade, it was just about bumping our dependencies and continuing to slowly adopt go modules. However, I overlooked that v2 changed some rules for ordering of flags and arguments. From the [upgrade guide](https://github.com/urfave/cli/blob/master/docs/migrate-v1-to-v2.md): > In v2 flags must come before args. This is more POSIX-compliant. You may need to update scripts, user documentation, etc. > > This will work: > > cli hello --shout rick > > This will not: > > cli hello rick --shout We're not keen on breaking our users scripts, so let's rollback to v1. There's an [open issue upstream](urfave/cli#1113) about the new ordering rules. We'll keep an eye on that, and maybe re-try the upgrade if it becomes possible to do so in a non-breaking way This is mostly a revert of PR #1233 and #1250, but with a few extra changes to things that happened after that PR (like adding the `artifact search` subcommand).
yob
force-pushed
the
rollback-ufave-cli-upgrade
branch
from
3e33d6a to
dc81adc
Compare
pda
approved these changes
Sep 9, 2020
There was a problem hiding this comment.
I have verified that for buildkite-agent meta-data get foo --default bar;
- v3.22.1 returns
"bar"with a warning, build success - v3.23.0 fails with 404 No key "foo" found, build fails
- this branch restores v3.22.1 behaviour; returns
"bar"with a warning, build success
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1285.
In #1233 we upgraded github.com/urfave/cli from v1 to v2. There was no functional need for the upgrade, it was just about bumping our dependencies and continuing to slowly adopt go modules.
However, I overlooked that v2 changed some rules for ordering of flags and arguments. From the upgrade guide:
We're not keen on breaking our users scripts, so let's rollback to v1. There's an open issue upstream about the new ordering rules. We'll keep an eye on that, and maybe re-try the upgrade if it becomes possible to do so in a non-breaking way
This is mostly a revert of PR #1233, but with a few extra changes to things that happened after that PR (like adding the
artifact searchsubcommand).