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

Font relative paths #52

Merged
merged 4 commits into from
Aug 20, 2017
Merged

Font relative paths #52

merged 4 commits into from
Aug 20, 2017

Conversation

djbe
Copy link
Member

@djbe djbe commented Aug 16, 2017

Fixes SwiftGen/SwiftGen#323.
Refs SwiftGen/SwiftGen#324.

templates related PR: SwiftGen/templates#71

  • Align templates to master before merge

@@ -84,6 +84,8 @@ public final class FontsParser: Parser {

public func parse(path: Path) {
let dirChildren = path.iterateChildren(options: [.skipsHiddenFiles, .skipsPackageDescendants])
let parentDir = path.absolute().parent()
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation?

Copy link
Member Author

Choose a reason for hiding this comment

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

For f. sake... Xode!

@djbe djbe force-pushed the feature/font-relative-paths branch from a71921b to c531812 Compare August 16, 2017 22:55
Copy link
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.

Wait, shouldn't we add some additional tests to ensure we won't have a regression regarding relative/absolute path in the future?

@djbe
Copy link
Member Author

djbe commented Aug 17, 2017

What do you want to test? The relative(to:) method?

@AliSoftware
Copy link
Contributor

Yeah and also try to call the while framework entry-point with a relative path and an absolute path and check they don't leak the full path in either case, something like that?

@AliSoftware
Copy link
Contributor

(Also reminder: don't forget to wait for SwiftGen/templates#71 then undo the Podfile changes before merging this)

@djbe
Copy link
Member Author

djbe commented Aug 17, 2017

That was in the templates repo, I already removed the change 👍

@AliSoftware
Copy link
Contributor

Haha I was actually searching it after I made that comment and failed to find it again and thought I needed some more sleep (which might still be true anyway 😜 )

@djbe
Copy link
Member Author

djbe commented Aug 17, 2017

Todo: align subrepo before merged

@djbe djbe force-pushed the feature/font-relative-paths branch from bc816fa to 51dec9a Compare August 20, 2017 15:38
@djbe djbe merged commit cdaad16 into master Aug 20, 2017
@djbe djbe deleted the feature/font-relative-paths branch August 20, 2017 15:43
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