-
-
Notifications
You must be signed in to change notification settings - Fork 619
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 validation for toolset. #1997
Conversation
@@ -174,6 +175,13 @@ | |||
end | |||
|
|||
|
|||
function m.actionSupportsToolset(prj) | |||
if not p.action.supportsToolset(prj.language, prj.toolset) then | |||
p.warn("Unsupported toolset '%s' used for language '%s' for project '%s'", prj.toolset, prj.language, prj.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most other validations use warn
and not error
...
So I stay coherent with above checks. (but I think all should be error
).
src/base/action.lua
Outdated
--- | ||
|
||
function action.supportsToolset(language, toolset) | ||
local function buildLanguageKeysMap() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how it should be done with extern registered languages. Apply to valid_tools
entry or registered action anyway...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this looks good. I'm a bit concerned that it might be emitting the warning on valid uses since we probably haven't been updating the valid_tools
along the way, but we can just make sure they're all fixed up before the next release.
0b0541b
to
716a57d
Compare
Tests actually fail as master fails. |
716a57d
to
504f6fc
Compare
What does this PR do?
Add validation for toolset.
So
valid_tools
is now useful.How does this PR change Premake's behavior?
Add a message for invalid configuration for toolset
Anything else we should know?
No.
Did you check all the boxes?
closes #XXXX
in comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!