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

fix: trim comments that are not modifers #371

Merged
decentral1se merged 2 commits from trim-env-var-comments into main 2023-10-09 12:44:18 +00:00
Conversation 3 Commits 2 Files Changed 12 +133 -26
decentral1se commented 2023-10-08 20:44:49 +00:00
Owner
Copy Link

See coop-cloud/organising#505. @cas i was thinking of just removing it altogether? As long as it isn't # length=... (or any future modifiers, which the code can adapt to). Does that seem OK?

See https://git.coopcloud.tech/coop-cloud/organising/issues/505. @cas i was thinking of just removing it altogether? As long as it isn't `# length=...` (or any future modifiers, which the code can adapt to). Does that seem OK?
decentral1se added 1 commit 2023-10-08 20:44:50 +00:00
fix: trim comments that are not modifers
All checks were successful
continuous-integration/drone/push Build is passing
Details
continuous-integration/drone/pr Build is passing
Details
be693e9df0 
See coop-cloud/organising#505
cas commented 2023-10-08 22:16:33 +00:00
Member
Copy Link

As long as the modifiers don't get into the values coming from the app config (.env) I'm good with them being non-stripped. I should've specified that this has been biting from the app config although I'm sure it has bitten from the compose.yml also.

As long as the modifiers don't get into the values coming from the app config (.env) I'm good with them being non-stripped. I should've specified that this has been biting from the app config although I'm sure it has bitten from the compose.yml also.
decentral1se commented 2023-10-09 06:42:12 +00:00
Author
Owner
Copy Link

Oh yeh, indeed, endless footguns on this one 😆 Will try to cover those edge cases.

Gonna close off coop-cloud/organising#496 with tihs as well.

Thanks for the heads up @cas!

Oh yeh, indeed, endless footguns on this one 😆 Will try to cover those edge cases. Gonna close off https://git.coopcloud.tech/coop-cloud/organising/issues/496 with tihs as well. Thanks for the heads up @cas!
decentral1se commented 2023-10-09 12:40:04 +00:00
Author
Owner
Copy Link

OK, this turned into a bit of a whack-a-mole but it seems like the simplest code change is just strip all comments when reading in the environment. Then, when we need comments, e.g. secret length modifiers, we re-read and don't strip. This is an easier mental model to follow in the code. Otherwise, I feel like comments will kinda just be popping up in unwanted places like in coop-cloud/organising#496.

OK, this turned into a bit of a whack-a-mole but it seems like the simplest code change is just strip all comments when reading in the environment. Then, when we need comments, e.g. secret length modifiers, we re-read and don't strip. This is an easier mental model to follow in the code. Otherwise, I feel like comments will kinda just be popping up in unwanted places like in https://git.coopcloud.tech/coop-cloud/organising/issues/496.
decentral1se force-pushed trim-env-var-comments from 7ad2e347ae to c249c6ae9c 2023-10-09 12:42:17 +00:00 Compare
decentral1se merged commit c249c6ae9c into main 2023-10-09 12:44:18 +00:00
decentral1se deleted branch trim-env-var-comments 2023-10-09 12:44:18 +00:00
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
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#371
No description provided.
Delete Branch "trim-env-var-comments"

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?