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

Add @system_wrapper.module_eval to defineinator, flaginator, so enviroment variables, #{ENV['VARIABLE']}, are parsed now. #880

Conversation

claudiiolima
Copy link

No description provided.

…oment variables, #{ENV['VARIABLE']}, are parsed now.
@claudiiolima claudiiolima changed the base branch from master to test/ceedling_0_32_rc April 22, 2024 17:38
@claudiiolima
Copy link
Author

This PR solves #879, so flags and defines, can be populate with enviroments variables now.

@mvandervoord
Copy link
Member

Thanks for making your changes against the release candidate! That makes it much easier for us to consider this PR!

@claudiiolima
Copy link
Author

Thanks for making your changes against the release candidate! That makes it much easier for us to consider this PR!

I tested it on my local project and everything worked fine.

This is my first contribution, so I hope everything goes well.

@mkarlesky
Copy link
Member

Thank you so much for this. I am going to reject the PR but still implement the changes in a coming pre-release version. Like path standardization and related tasks, this should all be collocated in the startup steps that process configuration before it is used. The string replacement in tool handling is some of the very oldest code in Ceedling. It's not a pattern that should be copied elsewhere. We'll centralize this during configuration processing. This will have the added benefit of producing configuration modifications you can see and verify by running the new dumpconfig command line.

@mkarlesky mkarlesky closed this May 10, 2024
@claudiiolima
Copy link
Author

Only to track modifications ✨ Ruby string replacement for :defines & :flags

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.

3 participants