-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Config helper #1073
Config helper #1073
Conversation
+ create helper function for simplest access Signed-off-by: Christoph Potas <christoph286@googlemail.com>
~ remove namespace from class name Signed-off-by: Christoph Potas <christoph286@googlemail.com>
Looks good. Please add docs and some tests for the class please, and then we are good to go. |
~ fix incorrect non-class files handling + add tests for all cases Signed-off-by: Christoph Potas <christoph286@googlemail.com>
Signed-off-by: Christoph Potas <christoph286@googlemail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide an example in the docs about using the config helper with a full namespace, and then I'll be happy to merge!
Signed-off-by: Christoph Potas <christoph286@googlemail.com>
Something is broken in the travis-ci unit tests.
Looking at the travis-ci results, it shows an error and four failures, but then echos "Hello World" at the end of the row after 1403/2089 and then exits with "success" !? |
ps - the error and two failures outside of ConfigTest show in other PRs, and were not caused by this PR. |
Hmm, the tests passed locally - it’s pretty weird, not sure what’s the different between our installation that can cause this. |
Are you perchance running on WIndows locally? |
Yes
…
On Jun 28, 2018 at 8:20 PM, <BCIT Instructor ***@***.***)> wrote:
Are you perchance running on WIndows locally?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#1073 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/ACUQbMDiUospEslrUhx5gwMXsZ_MuoQeks5uBR5YgaJpZM4Unjng).
|
Oops. Config::get('email') will return a class on your system because WIndows is case-insensitive. Linux won't find it, hence the test fails. |
Better practice is to test on Linux, eg inside Windows bash shell or inside a VM (eg VirtualBox), to avoid things that Windows will let slip through. I use Linux natively, and I think Lonnie uses VirtualBox + Vagrant on MacOSX. |
Oh, so i can’t handle lower case class names at all. |
The alternative is to UCfirst the class name that results, which should work on all platforms. |
It is not a classname case sensitivity, it is a filename case sensitivity problem. |
config helper implementation for #1071
Signed-off-by: Christoph Potas christoph286@googlemail.com