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

Vaultdep explicit secrets path #7

Merged
merged 2 commits into from
Aug 7, 2024
Merged

Conversation

aekblad
Copy link
Contributor

@aekblad aekblad commented Aug 7, 2024

What changes are made in this PR?

VaultDep now depends on the secrets_path path parameter and the app state only, without access to the entire request.

Why are these changes needed?

For readability, injected dependencies should be explicit and minimal. Deviating from that rule of thumb without a very good reason rapidly causes code to become hard to follow.

@aekblad aekblad requested a review from a team as a code owner August 7, 2024 09:31
@aekblad aekblad force-pushed the vaultdep-explicit-secrets-path branch from 5bf3157 to 4390114 Compare August 7, 2024 09:33
Copy link
Contributor

@zbleness zbleness left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@aekblad aekblad merged commit 9422175 into main Aug 7, 2024
10 checks passed
@aekblad aekblad deleted the vaultdep-explicit-secrets-path branch August 7, 2024 12:18
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.

2 participants