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

Remove unused configuration from gmake2 #2142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cathei
Copy link

@cathei cathei commented Sep 18, 2023

What does this PR do?

Remove env.PathVars (notice the capitalization) that had no effect.

How does this PR change Premake's behavior?

Shouldn't change any behavior as it has wrong name and not referenced.

Anything else we should know?

N/A

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

@nickclark2016
Copy link
Member

Any reason to remove this, rather than rename is properly to pathVars?

@cathei
Copy link
Author

cathei commented Sep 18, 2023

Any reason to remove this, rather than rename is properly to pathVars?

This was my thought process:

  • If the values are identical to tokens from fileconfig, then there would be no point of re-defining here.
  • If the values are not identical, then it might break existing scripts that use tokens, due to behavior change.

@Jarod42
Copy link
Contributor

Jarod42 commented Sep 18, 2023

As I remember, it might be useful only for cases with absolute = true (as other values are identical as without pathvars)...

But I would say our handling of absolute is strange anyway (we have %{!path} to turn/keep absolute path, %{absolute_path_token} might turn into relative path (inside %[path])).

Created project-tokens to test tokens.

@nickclark2016
Copy link
Member

@cathei do you mind pushing up an empty commit to retrigger actions? Once they're triggered and pass I should be able to merge this.

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