Upgrades, Tests #46
Upgrades, Tests #46
Conversation
|
I'd like to land the upgrades and fixes for existing functionality separately from new features. In other words, lets add |
|
@rwjblue sounds good to me. Any idea what's going on with the tests? |
|
Also, would you want the acceptance tests added before this PR should be considered 'done'? |
|
personally I'd land the upgraded blueprint stuff first, then additional tests, then any other fixes/work, then new features But I'm very grateful for the help any way you want to do it... |
|
CI seems to be having fits due to npm issues, can you make sure that |
|
@rwjblue there are blueprints? (or do you mean for MU stuff?) |
NullVoxPopuli
changed the title
|
yay the tests passed! |
|
so, @rwjblue what more would you like to see in this PR? |
MODULE_REPORT.md
Outdated
| @@ -0,0 +1,14 @@ | |||
| ## Module Report | |||
There was a problem hiding this comment.
Please remove this file (I'd prefer not to check it in)...
addon/helpers/is-active.js
Outdated
|
|
||
| compute(_params) { | ||
| let params = handleQueryParams(_params); | ||
| return this.get('router').isActive(...params); | ||
| }, | ||
|
|
||
| init() { |
There was a problem hiding this comment.
Please move init above compute...
addon/helpers/route-params.js
Outdated
|
|
||
| compute(params) { | ||
| return new RouteParams(this.get('router'), params); | ||
| }, | ||
|
|
||
| init() { |
There was a problem hiding this comment.
init should be the first method, can you move it up?
package.json
Outdated
| "test": "yarn ember test", | ||
| "test:all": "ember try:each", | ||
| "ember": "./node_modules/.bin/ember", | ||
| "ember:update": "./node_modules/.bin/ember-cli-update" |
package.json
Outdated
| "start": "yarn ember serve", | ||
| "test": "yarn ember test", | ||
| "test:all": "ember try:each", | ||
| "ember": "./node_modules/.bin/ember", |
There was a problem hiding this comment.
This is superfluous with yarn (you can run yarn ember without any changes to scripts).
There was a problem hiding this comment.
this is mindblowing. thank you. lol
package.json
Outdated
| "ember-cli-sri": "^2.1.0", | ||
| "ember-cli-uglify": "^1.2.0", | ||
| "ember-cli-uglify": "^2.0.0", | ||
| "ember-cli-update": "^0.23.0", |
There was a problem hiding this comment.
please remove, ember-cli-update should only be installed globally IMO
There was a problem hiding this comment.
aight. personally, because I have so many projects with different versions of things, I try to use local bins as much as possible
There was a problem hiding this comment.
Generally I feel the same way, but with ember-cli-update you never want an older version
Whenever it is used it should be installed again (to get all the bug fixes, new codemods, etc)...
There was a problem hiding this comment.
does ember-cli-update check itself for updates?
(maybe that's something I could submit a PR for if it doesn't already exist)
There was a problem hiding this comment.
I can't recall, but I think that was recently added....
|
@rwjblue changes applied |
Initial work for: emberjs/rfcs#339
Thanks to @MartinMalinda and @knownasilya for doing some additional work :)
TODO:
<Link />Component that has a default implementation similar to that of{{link-to}}(but with no positional params)<Link />component.