-
Notifications
You must be signed in to change notification settings - Fork 27
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
Minor fixes #64
Minor fixes #64
Conversation
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.
Thanks, looks good to me. Restarting CI to see if it passes. I think a test is flaky.
Still failing. After looking a bit closer, I think this may be due to dependency drift and us still testing against Phantom 1.x which I don't think we need to continue to do. We can likely drop this line: ember-asset-loader/.travis.yml Line 18 in f8f1a44
|
No big deal, these were just things I noticed when browsing the project. (Incidentally, spurious failures like this is why I disagree with the approach of not locking down dependencies in CI.) |
Noted (again) |
CI was fixed (thanks @lennyburdette!) in #66, this should pass with a rebase... |
The value after the type in a `@return` annotation is the description of the returned value, but this was being used as if it were a `@param`. Since `app` by itself is not a very good description, I just removed it.
Okay, rebased. |
Thank you @eventualbuddha! |
All pretty minor. Each commit fixes one thing.