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

Provide environment variables in configuration settings if present in… #226

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ZachWatkins
Copy link

… process.env

Change Summary:
[ What have you changed and why? ]

I have changed the extension to allow user configurations to include ${env:myVariable} in tag content definitions which will be replaced with environment variables available to the extension at runtime using process.env. This will be useful for automatic inclusion of a PHP project's version number which can be more easily automated as an environment variable than it can as a VS Code setting. Without this change I have to manually input the project's current version number for @since and @version tags. This also allows me to define more tag content at the User-level such as @copyright and @package by setting it to ${env:PACKAGE_NAME} and ensuring the directory provides this environment variable before VS Code starts.

Checks:

  • CHANGELOG.md updated with relevant changes

@neild3r
Copy link
Owner

neild3r commented Nov 11, 2022

Hmm I'm not sure why the workflows haven't run

@neild3r
Copy link
Owner

neild3r commented Nov 11, 2022

@ZachWatkins I know this is a late response but I've been extremely busy recently but would you be able to take a look at adding some unit tests for these additions.

In general I'm happy with what you are wanting to change we may want to think about where some of the code should sit. I did think that replacement set of classes might even be a good idea to implement where we do more of this sort of thing.

That shouldn't change the implementation of the unit tests though so let's look at getting those covered first.

@ZachWatkins
Copy link
Author

ZachWatkins commented Nov 13, 2022 via email

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.

None yet

2 participants