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

Adding a param to customize the access modifier #81

Closed
wants to merge 3 commits into from
Closed

Adding a param to customize the access modifier #81

wants to merge 3 commits into from

Conversation

txaiwieser
Copy link

This is very useful to being able to use SwiftGen from inside a framework. And use the generated code in the main target.

@AliSoftware
Copy link
Collaborator

AliSoftware commented Sep 27, 2017

Nice idea, this looks promising!

How about storing the {% if param.accessModifier %}{{ param.accessModifier}} {% endif %} block that you have to repeat in a lot of places… into a local Stencil variable using {% set access %}…{% endset %}?
This would allow you to use {{access}} directly all the time in all situations (if it's not set it's gonna be replaced with nothing (the empty string) and if it's set it's gonna be replaced by the access modifier and its space afterwards)

Also, don't forget to:

  • Apply the same logic to all templates, not just fonts/swift4
  • Update the unit tests too
  • credit yourself in the CHANGELOG!

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.

@txaiwieser Ping 😉
Would love to see this go forward. You just need to:

  • Replicate the same changes to all the templates (and not just fonts/swift4)
  • Add some unit tests for this param (just duplicate another unit test already testing another parameter and change the tested parameter for accessModifier ; you can then use the "Generate fixtures" scheme to generate the expected outputs and have all your tests ready for you)
  • and last but definitely not least: add a CHANGELOG entry to credit yourself!

@AliSoftware
Copy link
Collaborator

I've open #84 to supersede this PR with a slight different approach, as it seems this PR went a little stale.

AliSoftware added a commit that referenced this pull request Nov 1, 2017
Adding a `param` to customize the access modifier (supersedes #81)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants