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

WIP - Configurable Paths #152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

EvanLovely
Copy link
Member

Intended to fix #35 to bring in configurable paths besides just pointing towards source directory. This needs to be backwards compatible as well.

@EvanLovely
Copy link
Member Author

I've got a commit on this branch that cleans up the config setting so diffs should make much more sense now.

@EvanLovely
Copy link
Member Author

I could see using self::getOption() in Config.php as it utilizes a nice function from ArrayFinder that allows you to do self::getOption('paths.source.patterns') and not having to worry about if paths or paths.source is there before trying to get paths.source.patterns. Kind of like this:

- self::$options["patternSourceDir"] =
-   isset(self::$options["patternSourceDir"])
-     ? self::$options["sourceDir"] . DIRECTORY_SEPARATOR . self::cleanDir(self::$options["patternSourceDir"])
-     : self::$options["sourceDir"] . DIRECTORY_SEPARATOR . "_patterns";

+ self::$options["patternSourceDir"] =
+   self::getOption('paths.source.patterns')
+     ? self::$options["baseDir"] . self::cleanDir(self::getOption("paths.source.patterns"))
+     : self::$options["sourceDir"] . DIRECTORY_SEPARATOR . "_patterns";

@sghoweri sghoweri self-requested a review November 2, 2017 22:33
@sghoweri
Copy link
Contributor

sghoweri commented Nov 2, 2017

Oh wait a sec @EvanLovely are you just doing what I think you're doing here? Cleaning up the crappy whitespace with literally zero code changes otherwise?

If so, can we merge this down to develop (yay clean diffs) so we can point any subsequent PRs at that easier to follow base? Or maybe a separate PR so this can act as an umbrella ticket? shrug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants