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

No default lastKnownFileType #133

Merged
merged 1 commit into from
Nov 1, 2017
Merged

Conversation

toshi0383
Copy link
Contributor

Short description 📝

Reverts auto calculation of lastKnownFileType introduced in #58.

Solution 📦

It turns out that Xcode sets explicitFileType instead of lastKnownFileType for macOS app target automatically. So we shouldn't set default value for that.

        FR4799450001 /* LGTM.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = LGTM.app; sourceTree = BUILT_PRODUCTS_DIR; };

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Just a few comment. Would be great to get some input from @yonaskolb who introduced this change.

CHANGELOG.md Outdated
@@ -1,5 +1,9 @@
🚀 Check out the guidelines [here](https://github.com/xcodeswift/contributors/blob/master/CHANGELOG_GUIDELINES.md)

## master
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes are not necessary. We'll tag this PR with the proper milestone and we'll update the Changelog accordingly when we make the next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted👌

@@ -52,7 +52,7 @@ public class PBXFileReference: PBXObject, Hashable {
xcLanguageSpecificationIdentifier: String? = nil) {
self.fileEncoding = fileEncoding
self.explicitFileType = explicitFileType
self.lastKnownFileType = lastKnownFileType ?? path.flatMap { PBXFileReference.fileType(path: Path($0)) }
self.lastKnownFileType = lastKnownFileType
Copy link
Contributor

Choose a reason for hiding this comment

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

What are your thoughts on this @yonaskolb?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess if that' really the case. I can see why removing this might be good for other reasons as well, where we might be calculating the wrong lastKnownFileType as PBXFileReference.fileType( would never be perfect

@toshi0383 toshi0383 force-pushed the ts-no-default-lastknownfiletype branch from e143d41 to 18bef4b Compare October 30, 2017 20:35
@pepicrft pepicrft merged commit 2604759 into master Nov 1, 2017
@pepicrft pepicrft deleted the ts-no-default-lastknownfiletype branch November 1, 2017 09:48
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