Skip to content

TOML set_env file support #3478

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

Merged
merged 11 commits into from
Mar 6, 2025

Conversation

juditnovak
Copy link
Contributor

@juditnovak juditnovak commented Feb 5, 2025

This code change is addressing #3474, making sure that TOML config has the same support to apply set_env from file, as INI configuration does.

Resolves #3474

@juditnovak juditnovak force-pushed the fix/toml_setenv_from_file branch 2 times, most recently from 10c8e9c to 7dc1f78 Compare February 5, 2025 01:18
@juditnovak juditnovak marked this pull request as ready for review February 5, 2025 09:03
@juditnovak juditnovak changed the title [WIP] [FIX] TOML set_env from file [FIX] TOML set_env from file Feb 5, 2025
@gaborbernat
Copy link
Member

You still plan to tackle this?

@gaborbernat gaborbernat marked this pull request as draft February 10, 2025 19:06
@juditnovak
Copy link
Contributor Author

@gaborbernat Yes I do. Sorry for the silence, I was unavailable for a couple of days. Updated PR should be available early next week.

@juditnovak juditnovak force-pushed the fix/toml_setenv_from_file branch from 6fdde68 to 81a6997 Compare February 18, 2025 10:59
@gaborbernat
Copy link
Member

@juditnovak tag me when ready 👍

@juditnovak
Copy link
Contributor Author

@gaborbernat Sorry for the delay, the PR is ready. I keep thinking that I'm kind unhappy with the solution in terms of separation of concerns -- as part of the INI/TOML processing is happening on the level of each loader (__init__.py), while format-specific logic now is also hitting the shared (ideally generic) set_env module...
(Yet, otherwise set_env logic would need to "cascade up" to each loader's level :-/ )

In case I'm overlooking a different, more elegant solution, feel free to let me know, and I'm glad to adjust.

@juditnovak juditnovak marked this pull request as ready for review February 21, 2025 13:37
@gaborbernat gaborbernat changed the title [FIX] TOML set_env from file TOML set_env file support Feb 21, 2025
@juditnovak juditnovak force-pushed the fix/toml_setenv_from_file branch from 04b9290 to e4cabe8 Compare February 21, 2025 17:41
@juditnovak
Copy link
Contributor Author

@gaborbernat The condition in question is removed (assuming that 'revert' meant removal of newly added code).

@gaborbernat gaborbernat self-assigned this Feb 24, 2025
@gaborbernat
Copy link
Member

Yeah, didn't forget about this, have the tab pinned. Just this week has been very busy at work and didn't get time to allocate to this, we should get this out by early next week though 👍

@juditnovak
Copy link
Contributor Author

@gaborbernat Sure, thanks much for getting back.
No pressure, I just wanted to make sure to keep you in the loop :-)

@gaborbernat gaborbernat force-pushed the fix/toml_setenv_from_file branch 7 times, most recently from a683cfd to b339f50 Compare March 6, 2025 22:03
Signed-off-by: Bernát Gábor <[email protected]>
@gaborbernat gaborbernat force-pushed the fix/toml_setenv_from_file branch from b339f50 to 015cf6b Compare March 6, 2025 22:05
@gaborbernat gaborbernat enabled auto-merge (squash) March 6, 2025 22:07
@gaborbernat gaborbernat merged commit 4852262 into tox-dev:main Mar 6, 2025
28 checks passed
@gaborbernat
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TOML configuration of set_env should also support loading from environment files
2 participants