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 support for .swift file for xcode #1477

Merged

Conversation

sp-rafael-lecina
Copy link
Contributor

@sp-rafael-lecina sp-rafael-lecina commented Jul 7, 2020

What does this PR do?

  • Set swift files as source code
  • Adds a swiftversion to be able to configure Swift Language Version

How does this PR change Premake's behavior?

  • Adds .swift on the file extension lists with proper values
  • Adds a DSL api to set the SWIFT_VERSION setting

Are there any breaking changes? Will any existing behavior change?

  • No

Anything else we should know?

Add any other context about your changes here.

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

@starkos
Copy link
Member

starkos commented Jul 9, 2020

This looks like a nice feature addition, thanks for taking the time! Unfortunately, it looks like you've broken some of the existing unit tests with your changes—we need all tests to be passing before we can merge your change.

It might make things easier if you split your changes up into two PRs: one that adds Swift support, and another for the single value fix. The Swift changes should pass the existing tests with no trouble, but you will probably need to fix up some of them in order to pass your single value changes.

It would also be helpful if you could add a new test or two to cover your improvements, verify they work, and protect against any future regressions.

And a small GitHub tip: If you put "Closes #667" instead of "There was already an issue about this: #667" that issue will get closed automatically as soon as your PR merges.

You can push new commits to this branch to patch things up, or close this one and open new one(s). Seem reasonable?

@sp-rafael-lecina
Copy link
Contributor Author

Thanks
Once I have some time I'll do the fixes you suggest

Just one question. Do you know why the single value code was commented?

@starkos
Copy link
Member

starkos commented Jul 9, 2020

Sorry, no!

@sp-rafael-lecina
Copy link
Contributor Author

sp-rafael-lecina commented Jul 30, 2020

@starkos Don't actually know why CI failed on this and worked on this other one: #1491
Didn´t have much info when going to details

Copy link
Member

@samsinsane samsinsane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have unit tests for the new file format and API.

@samsinsane samsinsane merged commit a102474 into premake:master Aug 20, 2020
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.

Can this tool be updated to include and create .swift files?
4 participants