toolshed/abra
toolshed/abra
4
2
Fork 18
Code Issues 29 Pull Requests 3 Packages Releases 40 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: toolshed: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

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: change NewAppServerFlag usage description
All checks were successful
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

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
Member
Copy Link

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

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
Member
Copy Link

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

@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

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
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
decentral1se
p4u1
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
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
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#397
No description provided.
Delete Branch "wykwit/abra:minor-improvements"

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?