Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't allow LoadPath to be set via config file #1252

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Sep 16, 2024

This is a pseudo bug I introduced way back in #190 - it doesn't actually happen as our implementation always explicitly sets the property after we've successfully parsed the file into the struct, but it is possible for refactors like #1248 to reveal this and ultimately we don't want this behaviour so we might as well fix it.

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.29%. Comparing base (46ab63d) to head (f88f82e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1252      +/-   ##
==========================================
- Coverage   68.31%   68.29%   -0.02%     
==========================================
  Files         175      175              
  Lines       16764    16764              
==========================================
- Hits        11452    11449       -3     
- Misses       4680     4682       +2     
- Partials      632      633       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/config/config.go Outdated Show resolved Hide resolved
@G-Rath G-Rath force-pushed the config/fix-load-path branch 2 times, most recently from 13b71d3 to b0fbd6a Compare September 18, 2024 04:47
@another-rex another-rex merged commit 67a2f04 into google:main Sep 23, 2024
13 checks passed
@another-rex another-rex deleted the config/fix-load-path branch September 23, 2024 00:31
another-rex pushed a commit that referenced this pull request Sep 24, 2024
This behaviour changed as a result of both #1252 (which makes `LoadPath`
no longer a recognized config key) and #1249 (which causes the scanner
to error when a config has an unknown key), but they didn't directly
conflict with each other so got landed on `main` without this being
picked up

(fwiw I'm not worried about this being a reoccurring thing that's worth
guarding or preventing somehow as its usually pretty rare 🤷)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants