toolshed/abra
4
2
Fork 18
Code Issues 34 Pull Requests 3 Packages Releases 40 Activity

WIP: upgrade urfave/cli to v2 #404

Closed
p4u1 wants to merge 7 commits from p4u1/abra:upgrade-cli into main
pull from: p4u1/abra:upgrade-cli
merge into: toolshed:main
Conversation 5 Commits 7 Files Changed 50 +372 -292
p4u1 commented 2024-02-23 14:00:05 +00:00
Member
Copy Link

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
Some checks failed
continuous-integration/drone/pr Build is failing
Details
8a7d17f37b
decentral1se commented 2024-02-29 09:14:25 +00:00
Owner
Copy Link

@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
Owner
Copy Link

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 commented 2024-03-07 17:38:43 +00:00
Author
Member
Copy Link

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
p4u1 added 2 commits 2024-03-11 15:09:02 +00:00
Merge remote-tracking branch 'upstream/main' into upgrade-cli 114bdc5ce9
fix
Some checks failed
continuous-integration/drone/pr Build is failing
Details
7d80f4d56b
p4u1 added 1 commit 2024-03-11 15:14:39 +00:00
fmt
All checks were successful
continuous-integration/drone/pr Build is passing
Details
b81f5651d3
decentral1se commented 2024-04-06 22:21:41 +00:00
Owner
Copy Link

@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
Owner
Copy Link

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
All checks were successful
continuous-integration/drone/pr Build is passing
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
build
go build related issues
ci/cd
Building things with CI/CD
contributing
Dealing with contributing
critical fix
https://docs.coopcloud.tech/federation/resolutions/passed/010/
design
UI/UX
documentation
Documenting all the things
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
meta
relating to the project or repo itself
question
More information is needed
red abya yala
Work funded by Calyx institute for Red Abya Yala
release
Release management
release-candidate
Related to the new release candidate
security
Security related
test
Unit/integration testing
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
3wordchant abra-bot ammaratef45 Apfelwurm carla cas decentral1se fauno kawaiipunk knoflook lambdabundesverband moritz p4u1 simon trav val yksflip
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#404
No description provided.
Delete Branch "p4u1/abra:upgrade-cli"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?