-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Feature/379 update blueprint #380
base: master
Are you sure you want to change the base?
Feature/379 update blueprint #380
Conversation
5f14c3c
to
49dd10d
Compare
package.json
Outdated
@@ -68,5 +68,8 @@ | |||
}, | |||
"ember-addon": { | |||
"configPath": "tests/dummy/config" | |||
}, |
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.
I don't think we should add the edition in the package.json until we update the version of ember-cli (we're currently on ember-cli 3.3)
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.
FYI I've started trying to upgrade ember-cli in #385
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.
@Gorzas if you pull the latest changes from master you should be able to remove these changes now
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.
Just some final touch ups and we should be good to go 💪
|
||
export default MF.Fragment.extend({ | ||
export default class <%= classifiedModuleName %> extends Fragment { | ||
<%= attrs %> |
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.
Could we remove the space here because having it indented results in the first attr
being indented too far?
<%= attrs %> | |
<%= attrs %> |
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.
@patocallaghan doing some tests with the generator, I've found that the result with attributes isn't valid. Look what happens when I type ember g fragment foo bar:string
:
import { attr } from '@ember-data/model';
import Fragment from 'ember-data-model-fragments/fragment';
export default class Foo extends Fragment {
bar: attr('string')
}
So I'm going to fix that too.
package.json
Outdated
@@ -68,5 +68,8 @@ | |||
}, | |||
"ember-addon": { | |||
"configPath": "tests/dummy/config" | |||
}, |
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.
@Gorzas if you pull the latest changes from master you should be able to remove these changes now
49dd10d
to
23326ff
Compare
No description provided.