toolshed/abra
4
3
Fork 21
Code Issues 40 Pull Requests 4 Packages Releases 42 Activity

Add -n and -m to recipes/upgrade #291

Merged
cas merged 5 commits from cr_upgrade_noninnteractive into main 2023-04-27 16:46:01 +00:00
Conversation 2 Commits 5 Files Changed 1 +69 -8
cas commented 2023-04-13 16:35:10 +00:00
Member
Copy Link

Support the no-input flag and the machine-readable-output flag in the recipe upgrade command.

Support the no-input flag and the machine-readable-output flag in the `recipe upgrade` command.
cas added 4 commits 2023-04-13 16:35:25 +00:00
recipe/upgrade: Add non-interactive mode.
All checks were successful
continuous-integration/drone/push Build is passing
Details
2a82936caa 
Add support for -n which just outputs the list of compatible tags for each image.
recipe/upgrade: Refactor upgradability list to make output easier
All checks were successful
continuous-integration/drone/push Build is passing
Details
fd7efa7daf 
For future, we can print the struct as JSON.
Add machine output for recipe/upgrade
All checks were successful
continuous-integration/drone/push Build is passing
Details
2fb490c8b3 
- Normal faff related to calling external libraries with structs thnx go
- Ouputs json now
Make -m imply -n in recipe/upgrade
All checks were successful
continuous-integration/drone/push Build is passing
Details
continuous-integration/drone/pr Build is passing
Details
196905526a
3wordchant requested review from 3wordchant 2023-04-13 16:39:29 +00:00
3wordchant approved these changes 2023-04-13 16:40:33 +00:00
decentral1se reviewed 2023-04-14 11:44:31 +00:00
decentral1se left a comment
Owner
Copy Link

Nice one, thanks! Some minor comments.

Nice one, thanks! Some minor comments.
cli/recipe/upgrade.go
@ -27,6 +28,13 @@ type imgPin struct {
version tagcmp.Tag
}
type anUpgrade struct {
decentral1se commented 2023-04-14 11:38:51 +00:00
Owner
Copy Link

Missing doc string? E.g.

// anUpgrade represents ...
Missing doc string? E.g. ```go // anUpgrade represents ... ```
cas marked this conversation as resolved
cli/recipe/upgrade.go
@ -249,0 +261,4 @@
Service: service.Name,
Image: image,
Tag: tag.String(),
UpgradeTags: make([]string, len(compatibleStrings[1:])),
decentral1se commented 2023-04-14 11:41:41 +00:00
Owner
Copy Link

Maybe sticking len(compatibleStrings[1:]) in a named variable will be more readable since it is used twice. The 1:... is sure to not stack trace since there is always 1 entry in it? We could comment that out also.

Maybe sticking `len(compatibleStrings[1:])` in a named variable will be more readable since it is used twice. The `1:...` is sure to not stack trace since there is always 1 entry in it? We could comment that out also.
cas commented 2023-04-27 16:44:20 +00:00
Author
Member
Copy Link

The len() is less useful than the slice, I've made that change.

The `len()` is less useful than the slice, I've made that change.
cas marked this conversation as resolved
cli/recipe/upgrade.go
@ -252,0 +278,4 @@
} else {
prompt := &survey.Select{
Message: msg,
Help: "enter / return to confirm, choose 'skip' to not upgrade this tag, vim mode is enabled",
decentral1se commented 2023-04-14 11:42:39 +00:00
Owner
Copy Link

Should we make this a const somewhere? Think it's getting re-used in several places now.

Should we make this a `const` somewhere? Think it's getting re-used in several places now.
cas commented 2023-04-27 16:44:44 +00:00
Author
Member
Copy Link

We should do but I don't want to muddle this in with the change.

We should do but I don't want to muddle this in with the change.
cas marked this conversation as resolved
cli/recipe/upgrade.go
@ -268,0 +308,4 @@
jsonstring, err := json.Marshal(upgradeList)
if err != nil {
logrus.Fatal(err)
} else {
decentral1se commented 2023-04-14 11:43:48 +00:00
Owner
Copy Link

This is a dangling else, it can be dropped?

This is a dangling else, it can be dropped?
cas commented 2023-04-27 16:45:05 +00:00
Author
Member
Copy Link

yep! done

yep! done
cas marked this conversation as resolved
cli/recipe/upgrade.go
@ -268,0 +312,4 @@
fmt.Println(string(jsonstring))
}
} else {
decentral1se commented 2023-04-14 11:44:15 +00:00
Owner
Copy Link

For readability, this else could be also dropped if you return earlier in the above block. Up to you.

For readability, this `else` could be also dropped if you `return` earlier in the above block. Up to you.
cas commented 2023-04-27 16:45:35 +00:00
Author
Member
Copy Link

Yep, does looks cleaner. Done.

Yep, does looks cleaner. Done.
cas marked this conversation as resolved
cas added 1 commit 2023-04-27 16:44:08 +00:00
Add some minor tweaks to machine readable pathway in recipe upgrade
Some checks failed
continuous-integration/drone/pr Build is failing
Details
continuous-integration/drone/push Build is passing
Details
7c55325c13
cas merged commit c5d9d88359 into main 2023-04-27 16:45:59 +00:00
cas deleted branch cr_upgrade_noninnteractive 2023-04-27 16:46:20 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
3wordchant
Labels
Clear labels
bug
Something is not working
build
go build related issues
ci/cd
Building things with CI/CD
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
easy-first-issue
Something for new people to get stuck into. We hope it's easy!
enhancement
New feature
help wanted
Need some help
i10n
Everything to do with localisation
i18n
Everything to do with internationalisation
installer
Everything to do with the install script.
invalid
Something is wrong
question
More information is needed
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 aadil abra-bot ammaratef45 Apfelwurm carla cas coopcloud cyrnel decentral1se fauno iexos jade jmakdah2 joe-irving kawaiipunk knoflook kolaente lambdabundesverband linnealovespie moritz notplants p4u1 pharaohgraphy renovate-bot ripclap simon sixsmith trav val yksflip
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#291
No description provided.
Delete Branch "cr_upgrade_noninnteractive"

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?