WIP: upgrade urfave/cli to v2 #404

2024-02-23T14:00:05Z

p4u1 commented

2024-02-23 14:00:05 +00:00

THIS IS VERY WIP!!!!

This is an attempt to fix a regression found by running the integration test suite.

The problem is with running the following:

  run $ABRA app cmd --local "$TEST_APP_DOMAIN" test_cmd_arg -- bing

This does not work properly with the old cli library

The main breaking change is a change in the order of flag and argument parsing (see here for details in the migration guide). This change is also what fixes the problem above.

THIS IS VERY WIP!!!! This is an attempt to fix [a regression](https://git.coopcloud.tech/coop-cloud/abra/pulls/403#issuecomment-19723) found by running the integration test suite. The problem is with running the following: ``` run $ABRA app cmd --local "$TEST_APP_DOMAIN" test_cmd_arg -- bing ``` This does not work properly with the old cli library The main breaking change is a change in the order of flag and argument parsing ([see here](https://cli.urfave.org/migrate-v1-to-v2/#flags-before-args) for details in the migration guide). This change is also what fixes the problem above.

p4u1 added 4 commits 2024-02-23 14:00:06 +00:00

ugrade urfave/cli to v2 428426b6b7

partially fix tests/integration/app_cmd.bats ac39d6ab97

wip try to get tests running deb4293fba

fix

continuous-integration/drone/pr Build is failing

Details

8a7d17f37b

decentral1se commented

2024-02-29 09:14:25 +00:00

@p4u1 Trying to dig into this now. abra users at large requested that we stay on v1 of urfave for the arg handling that it provides and I did the revert migration in 0e688f1407. Maybe handy if you DM me on matrix to figure this out together. Thanks for looking into it!

@p4u1 Trying to dig into this now. `abra` users at large requested that we stay on v1 of `urfave` for the arg handling that it provides and I did the revert migration in 0e688f1407a1f17333ea6cf4c4f64ba9b9d800fa. Maybe handy if you DM me on matrix to figure this out together. Thanks for looking into it!

decentral1se commented

2024-02-29 09:18:00 +00:00

coop-cloud/organising#336 seems relevant? Which involved dropping back to a point release of urfave? We could possibly attempt to pin there... am using git log -p --follow go.mod to check the updates/commits...

https://git.coopcloud.tech/coop-cloud/organising/issues/336 seems relevant? Which involved dropping back to a point release of `urfave`? We could possibly attempt to pin there... am using `git log -p --follow go.mod` to check the updates/commits...

p4u1 referenced this pull request from toolshed/organising

2024-03-07 17:37:42 +00:00

Non ambigous Argmuent and Flag position #581

p4u1 commented

2024-03-07 17:38:43 +00:00

Thanks for the feedback! I open an issue to discuss this further coop-cloud/organising#581

Thanks for the feedback! I open an issue to discuss this further https://git.coopcloud.tech/coop-cloud/organising/issues/581

decentral1se referenced this pull request from toolshed/organising

2024-03-09 08:51:46 +00:00

Non ambigous Argmuent and Flag position #581

p4u1 added 2 commits 2024-03-11 15:09:02 +00:00

Merge remote-tracking branch 'upstream/main' into upgrade-cli 114bdc5ce9

fix

continuous-integration/drone/pr Build is failing

Details

7d80f4d56b

p4u1 added 1 commit 2024-03-11 15:14:39 +00:00

fmt

continuous-integration/drone/pr Build is passing

Details

b81f5651d3

decentral1se commented

2024-04-06 22:21:41 +00:00

@p4u1 given coop-cloud/docs.coopcloud.tech#256 (comment) I think it might be worth a look into how feasible it is to upgrade further into the alpha v3 which provides https://github.com/urfave/cli/pull/1568. this would allow us to specify some flags ("--chaos", "--debug", etc.) as global and can be passed in any "style"? This is a bit of a long shot but might be nicer than "patch to re-order flags" and we could also help upstream fix stuff as we go... whatcha reckon?

@p4u1 given https://git.coopcloud.tech/coop-cloud/docs.coopcloud.tech/pulls/256#issuecomment-20322 I think it might be worth a look into how feasible it is to upgrade further into the alpha v3 which provides https://github.com/urfave/cli/pull/1568. this would allow us to specify some flags ("--chaos", "--debug", etc.) as global and can be passed in any "style"? This is a bit of a long shot but might be nicer than "patch to re-order flags" and we could also help upstream fix stuff as we go... whatcha reckon?

decentral1se commented

2024-07-09 12:51:53 +00:00

I've got a spike coming which upgrades to v3 and maintains the current v1 re-ordering (the goal: similar level of broken but we get the improvements that have been baked into urfave/cli over 2 new versions... via https://github.com/urfave/cli/pull/1928) 🤸 Wil close this off for now

I've got a spike coming which upgrades to v3 and maintains the current v1 re-ordering (the goal: similar level of broken *but* we get the improvements that have been baked into `urfave/cli` over 2 new versions... via https://github.com/urfave/cli/pull/1928) 🤸 Wil close this off for now

❤️ 1

decentral1se closed this pull request

2024-07-09 12:51:53 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

This pull request is marked as a work in progress.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: toolshed/abra#404