Skip to content
This repository was archived by the owner on Sep 6, 2018. It is now read-only.

Ensure the asset catalog parser ignores types we don't support (for now)#7

Merged
AliSoftware merged 3 commits intomasterfrom
feature/ac-ignore-unsupported-types
Feb 13, 2017
Merged

Ensure the asset catalog parser ignores types we don't support (for now)#7
AliSoftware merged 3 commits intomasterfrom
feature/ac-ignore-unsupported-types

Conversation

@djbe
Copy link
Copy Markdown
Member

@djbe djbe commented Jan 31, 2017

Fixes #6.

We should consider eventually supporting the other types. For example: use setAlternateIconName(_:completionHandler:) for app icon sets.

appIconSet, dataSet, gameCenterDashboardImage, gameCenterLeaderboard,
gameCenterLeaderboardSet, iconSet, imageSet, imageStack, launchImage,
spriteAtlas, stickerPack
]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more readable to do a whitelist rather than a blacklist?

  1. If someone wants to make a PR to support an additional type, if would be more logical to me to let them add a type rather than remove one from that list
  2. If a new type comes with the next version of iOS and Xcode, wth a whitelist we won't have to ad that new type to the list until we support it

spriteAtlas, stickerPack
]

static let supported = [imageSet]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe just add a comment here to tell that this is the list of folder extensions in the xcassets folder that we know how to parse, so that in the future when people will want to add PRs to support more data type (I know I have a coworker wishing to add SpriteKit support soon for example) they won't forget to add them here.

Now that the whitelist is only one item (compared to the blacklist listing a lot of extensions) any contributor won't have a lot of examples to get inspiration from like we had with the blacklist version, so better documentation would help instead.

Copy link
Copy Markdown
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Once the comment is added, feel free to merge 😉

AliSoftware pushed a commit to SwiftGen/templates that referenced this pull request Feb 13, 2017
@AliSoftware AliSoftware merged commit c7896a3 into master Feb 13, 2017
@djbe djbe deleted the feature/ac-ignore-unsupported-types branch February 13, 2017 17:07
@djbe djbe modified the milestone: 1.0.0 Feb 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants