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

Hotfix/expose image name #85

Merged
merged 6 commits into from
Oct 26, 2017

Conversation

abbeyjackson
Copy link
Contributor

Change private access to private(set) in order to expose image name on swift2.stencil, swift3.stencil and swift4.stencil for the xcassets folder.

@AliSoftware
Copy link
Collaborator

Thanks for the PR!

Don't forget to:

  • Update the Unit Tests. You can do that by opening the xcworkspace, selecting the "Generate Output" scheme, and hit Cmd-U. (†)
  • Add an entry in the CHANGELOG to credit yourself (be sure to respect the same format as the other entries, especially the two spaces after the period at the end of the description line

(†) This will run the tests, which use your templates to generate the outputs… then instead of comparing that generated output to the expected files like normal tests do with the "Tests" scheme, the "Generate Output" scheme will end up overriding the expected output files with the generated output, to update them. This way, you don't have to update all fixtures for the unit tests manually! Run the Generate Output and let it do the work for you 😉

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.

  • Update unit tests (by just hitting Cmd-U on the "Generate Output" scheme in the Xcode workspace)
  • Add a CHANGELOG entry to log the change and credit yourself

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 little adjustment for the CHANGELOG, otherwise LGTM!

CHANGELOG.md Outdated
@@ -20,6 +20,14 @@ _None_

_None_

## 2.2.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move your entry under ## Master — that's what it's there for — rather than assuming that the next version will be a 2.2.1 (we might not release a new version immediately so maybe there will be other PRs by other people that will make us need to bump the minor version when we decide to release) 😉

@AliSoftware
Copy link
Collaborator

@abbeyjackson Sorry for the inconvenience, but it happens that we just realised an issue with our CI vs Xcode 9, which I just fixed recently on master… but wasn't fixed at the time you opened your PR.

So if you could rebase your branch on top of the master branch, that will allow you to include that CircleCI fix and avoid the CI to fail on your PR because of this issues which would have nothing to do with your code

@abbeyjackson
Copy link
Contributor Author

@AliSoftware okay I'll have to do this back at home tonight since I don't think you can rebase via the github website can you...?

@AliSoftware
Copy link
Collaborator

No sadly you can't rebase via the web interface 😢

Change private access to private(set) in order to expose image name.
Change fileprivate access to fileprivate(set) in order to expose image name.
Change fileprivate access to fileprivate(set) in order to expose image name.
Moved entry to Master section on Changelog
@abbeyjackson
Copy link
Contributor Author

abbeyjackson commented Oct 26, 2017

@AliSoftware just got my magic goblin to do it! hah!

@AliSoftware AliSoftware merged commit 1ae3777 into SwiftGen:master Oct 26, 2017
@diederich
Copy link
Contributor

Nice, just found this when I found out that the name of the color is fileprivate as well. Should we change that too?

@AliSoftware
Copy link
Collaborator

AliSoftware commented Oct 27, 2017

@diederich I didn't realize people had a use for accessing the string name of those assets, hence them being fileprivate so far.

But indeed now that you talk about it, for consistency we should definitely apply the same change to color templates too! Would welcome a PR 😉

@diederich
Copy link
Contributor

Yeah, I stumbled over the allColors array, and just hacked a TableVC together to show the Colors that's used in an app. And there the name would indeed be nice. I'll look into it..

diederich added a commit to diederich/templates that referenced this pull request Oct 30, 2017
allow access to the name of the color asset,
e.g. to show a list of all colors used in the app.
See SwiftGen#85 for the same on images.
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.

3 participants