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

Load environment variables as a resolver #480

Merged
merged 1 commit into from
Feb 9, 2025

Conversation

IxDay
Copy link
Contributor

@IxDay IxDay commented Dec 16, 2024

This is a draft as a discussion starting point. I need to add comments, and maybe simplify code a bit.

Closes: #477

@IxDay
Copy link
Contributor Author

IxDay commented Jan 1, 2025

Just in case you pass by. I made some adjustments regarding your comments

@IxDay
Copy link
Contributor Author

IxDay commented Feb 5, 2025

Bumping just in case

Copy link
Owner

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

I see your comment calls out applying to args, which I don't love special casing here to be honest, but in lieu of a better API, I definitely prefer this to hard wiring environment application.

Thanks!

@IxDay
Copy link
Contributor Author

IxDay commented Feb 9, 2025

it needed a rebase and minor fixes for the linter, can you approve once more?

Results of the CI here: https://github.com/IxDay/kong/actions/runs/13225890198

@IxDay IxDay requested a review from alecthomas February 9, 2025 14:24
@alecthomas alecthomas merged commit 3cedc44 into alecthomas:master Feb 9, 2025
5 checks passed
@alecthomas
Copy link
Owner

This is awesome, thanks @IxDay :)

@alecthomas
Copy link
Owner

Tagged v1.8.0

alecthomas added a commit that referenced this pull request Feb 13, 2025
@alecthomas
Copy link
Owner

@IxDay I had to revert this due to two (#497 and #498) issues being reported. I'm happy to give it another shot if you want to, but up to you.

@IxDay
Copy link
Contributor Author

IxDay commented Feb 17, 2025

I will take a look

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.

env annotation is triggered before the BeforeResolve hook
2 participants