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

[Xcodeproj] Remove serializeArray method, use joined(separator:) instead #330

Merged
merged 1 commit into from
May 9, 2016

Conversation

aciidgh
Copy link
Member

@aciidgh aciidgh commented May 8, 2016

@czechboy0
Copy link
Contributor

Awesome, just hit this today too!

@dsperling
Copy link

Confirmed that SR-1441 is fixed. Thanks.

Another problem in Kitura has been uncovered with this swift-build update with the use of PRODUCT_MODULE_NAME = '$(TARGET_NAME:c99extidentifier)';"

This causes a swift compile error with "import KituraTemplateEngine" as the xcodeproj creates the module name as "Kitura_TemplateEngine". This is a separate issue so I will open a new ticket once it can be determined if the issue is in Package Manager or Kitura-TemplateEngine-0.9.0.

@aciidgh
Copy link
Member Author

aciidgh commented May 8, 2016

@ddunbar I don't think this one requires CI either but upto you.
@dsperling I ran into that issue today with your sample package, its most certainly swiftpm's bug, I didn't have enough time to fix it today but will do soon! please file a bug if you have time.

@dsperling
Copy link

Created this ticket for the "Kitura_TemplateEngine" issue.
https://bugs.swift.org/browse/SR-1444

@ddunbar
Copy link
Member

ddunbar commented May 9, 2016

LGTM, but I think we should have test coverage of the basic scenario in play here.

@aciidgh
Copy link
Member Author

aciidgh commented May 9, 2016

is it fine if tests for few of the fixes recently come together in a separate PR ?

@ddunbar
Copy link
Member

ddunbar commented May 9, 2016

Yeah, that is fine with me. For things like this, I often try and ensure one simple test which covers "basic sanity" and then extend with specific tests for regressions as they appear.

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

4 participants