toolshed/abra
4
4
Fork 23
Code Issues 82 Pull Requests 12 Packages Releases 45 Activity

WIP: minor improvements #397

Closed
wykwit wants to merge 7 commits from wykwit/abra:minor-improvements into main
pull from: wykwit/abra:minor-improvements
merge into: :main
Conversation 5 Commits 7 Files Changed 16
+63 -68
wykwit commented 2024-01-06 01:25:03 +00:00
First-time contributor
Copy Link
Copy Source

Minor improvements to make things easier to understand and make the staticcheck slightly less sad.

Minor improvements to make things easier to understand and make the `staticcheck` slightly less sad.
wykwit added 7 commits 2024-01-06 01:25:03 +00:00
chore: use keyed fields with struct literals 108456585d
chore: list upgrade sources in cmd help 39c531b9e8
fix: display text next to emojis in app check cmd 3fd249b0d8
chore: use recipePkg import consistently 192d430039
chore: use Printf() instead of Println(Sprintf()) 0c4d5b2e69
chore: remove unneccessary call to Sprintf 700e951e5e
chore: change NewAppServerFlag usage description
continuous-integration/drone/pr Build is passing
Details
d3022c27ea
decentral1se approved these changes 2024-01-06 09:12:54 +00:00
decentral1se left a comment
Owner
Copy Link
Copy Source

LGTM @wykwit, thanks! Merge when you're happy with it 💘

LGTM @wykwit, thanks! Merge when you're happy with it 💘
p4u1 reviewed 2024-01-06 15:57:31 +00:00
cli/app/check.go
@@ -6,4 +6,3 @@
"coopcloud.tech/abra/pkg/config"
"coopcloud.tech/abra/pkg/formatter"
"coopcloud.tech/abra/pkg/recipe"
recipePkg "coopcloud.tech/abra/pkg/recipe"
p4u1 commented 2024-01-06 15:57:03 +00:00
Owner
Copy Link
Copy Source

I think "coopcloud.tech/abra/pkg/recipe" should be the preferd way to import the package. Or is there a specific reason why you changed the import everywhere?

I think `"coopcloud.tech/abra/pkg/recipe"` should be the preferd way to import the package. Or is there a specific reason why you changed the import everywhere?
wykwit commented 2024-01-06 16:17:32 +00:00
Author
First-time contributor
Copy Link
Copy Source

I changed it wherever there was a double import, because in some places recipe can either be a variable or a package. It was used inconsistently within those source files, referring to the package sometimes as recipePkg and sometimes as just recipe which is additionally confusing when recipe is also used there as a common variable name. There are still some other places where the import is not renamed, but when the package name gets overshadowed it's not used anymore.

I changed it wherever there was a double import, because in some places `recipe` can either be a variable or a package. It was used inconsistently within those source files, referring to the package sometimes as `recipePkg` and sometimes as just `recipe` which is additionally confusing when `recipe` is also used there as a common variable name. There are still some other places where the import is not renamed, but when the package name gets overshadowed it's not used anymore.
p4u1 approved these changes 2024-01-06 15:58:03 +00:00
p4u1 left a comment
Owner
Copy Link
Copy Source

Added a comment about the import path, otherwise LGTM :)

Added a comment about the import path, otherwise LGTM :)
decentral1se commented 2024-03-27 05:41:38 +00:00
Owner
Copy Link
Copy Source

@wykwit got any cycles to take another run at this?

@wykwit got any cycles to take another run at this?
decentral1se changed title from Minor improvements to WIP: minor improvements 2024-03-27 05:43:08 +00:00
decentral1se commented 2024-06-24 22:08:49 +00:00
Owner
Copy Link
Copy Source

Gonna close off due to inactivity.

Gonna close off due to inactivity.
decentral1se closed this pull request 2024-06-24 22:08:49 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
This pull request is marked as a work in progress.
Sign in to join this conversation.
Reviewers
No Reviewers
decentral1se
p4u1
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
 tech-debt
test
Unit/integration testing
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
3wordchant aadil (Aadil Ayub) abra-bot (Abra Bot) ammaratef45 amras (Sarma) Apfelwurm BornDeleuze Brooke carla cas (Cassowary) coopcloud cyrnel decentral1se (d1) dede devydave fauno (fauno) iexos jade (Jade Ambrose) jjsfunhouse jmakdah2 (Jackie Makdah) joe-irving (Joe Irving) kawaiipunk (KawaiiPunk) knoflook kolaente lambdabundesverband linnealovespie (April) moosemower moritz notplants oxaliq (sorrel) p4u1 pharaohgraphy (Andrew 🐦‍🔥❤️‍🔥✴️) renovate-bot (Comrade Renovate Bot) ripclap simon sixsmith (Sixsmith) stevensting trav (Trav Fryer) val (val (he/him)) yksflip
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#397