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

Fonts: parameters to change typealias name & struct name #728

Merged
merged 6 commits into from
Jul 6, 2020

Conversation

djbe
Copy link
Member

@djbe djbe commented Jun 23, 2020

Fixes #647.

Mirrors the implementation from the xcassets templates.

@djbe djbe added this to the 6.3.0 milestone Jun 23, 2020
@SwiftGen-Eve
Copy link

SwiftGen-Eve commented Jun 23, 2020

Hey 👋 I'm Eve, the friendly bot watching over SwiftGen 🤖

Thanks a lot for your contribution!


Seems like everything is in order 👍 You did a good job here! 🤝

Generated by 🚫 Danger

@djbe djbe force-pushed the feature/font-alias branch 2 times, most recently from 2d550a0 to 4e30429 Compare June 24, 2020 16:05
@djbe djbe force-pushed the feature/font-alias branch 2 times, most recently from 122f535 to 0a5a99c Compare July 5, 2020 21:42
@AliSoftware
Copy link
Collaborator

I don't think this is the best approach, because in other templates we've migrated away from those top-level typealiases and moved them inside the top-level enum (see here for example)

So I think a better approach would be to remove those top-level Font type aliases and to instead define them inside the enum {{enumName|default:"FontFamily"}}. That way we avoid the risk of conflict between SwiftUI.Font and FontFamily.Font.

@djbe djbe changed the title Fonts: parameter to change typealias name Fonts: parameters to change typealias name & struct name Jul 6, 2020
@djbe
Copy link
Member Author

djbe commented Jul 6, 2020

You're right, why did I not think of that 🤦
I've reworked the PR as follows:

  • parameter to change struct name (like xcassets)
  • move platform typealias into struct (like xcassets)
  • parameter to change typealias name (like xcassets)
  • deprecate typealias (like xcassets)

Note that I've rebased all commits, as this is a full rework of the PR.

Copy link
Collaborator

@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.

just a conjugation typo and indent Q otherwise LGTM 👌

CHANGELOG.md Show resolved Hide resolved
Documentation/templates/fonts/swift4.md Show resolved Hide resolved
templates/fonts/swift4.stencil Show resolved Hide resolved
@djbe djbe merged commit b96c28c into develop Jul 6, 2020
@djbe djbe deleted the feature/font-alias branch July 6, 2020 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants