-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add pod blueprint generators #1994
Conversation
f7b7602
to
f5617ce
Compare
@jgwhite - Can you review and +1/-1? You know this area of the code much better than I.... |
f5617ce
to
499bea9
Compare
Rebased with updated index.js for blueprints (no longer using Blueprint.extend()). |
var locals = { dasherizedModuleName: stringUtils.dasherize(moduleName) }; | ||
this.pod = options.pod; | ||
this.podPath = generatePodPath(); | ||
this.hasPath = hasPathToken(this.files()); | ||
this.project = options.project; | ||
|
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.
Cosmetic: align =
❤️ This is an awesome piece of work. ❤️ I’ve made a couple of cosmetic comments more to aid myself absorbing all the changes than anything else. Some thoughts: podModulePrefixI believe this should be defined in @rwjblue @stefanpenner — test tokenCould the WarningsNow that test blueprints have been separated, the “this blueprint does not support pods” warning is a bit noisy. Can we add a flag to silence these warnings? This would allow the parent blueprint to silence its child’s warnings. |
@jgwhite - You didn't miss it, I just did it per your request above (seemed like a good idea). |
@jgwhite - I will get right on these, shouldn't be too tough to get rebased and make the changes. |
0934b73
to
c90f70f
Compare
The podModulePrefix is now pulled in from config, and the podPath is built in I went through the blueprint files that used the |
Should I go through and rename the test files that currently use |
Leave the non-pod test blueprints as-is. I’ll give this another review later tonight. |
I'm currently watching #2027 for changes that I'll need to apply to my |
👍 |
ef85db3
to
732a6d3
Compare
732a6d3
to
79bcc17
Compare
@rwjblue - Done! |
79bcc17
to
df18cdc
Compare
One more review today. If there's nothing outstanding then we'll be good to go. |
The fileMapToken order should match fileMapTokenValues order | ||
if overriding. The fileMapTokenValues order should match fileMapTokens | ||
order. Default is expected, pods is necessary if pod structure | ||
is supported for blueprint. |
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.
This explanation is a little confusing. Where is fileMapTokenValues
specified? Could you give this inline doc a little rewrite?
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.
Whoops, good catch, this documentation is from an earlier version of the method.
Aside from that one comment on the inline documentation this is a home run. Tweak that, add a CHANGELOG entry, and we’re good to merge. Thanks for putting so much time and consideration into this. |
Note: the changelog entry should go under a |
`fileMapTokens` to blueprints.
df18cdc
to
e8a2555
Compare
LGTM |
Introduces the option for using a pod structure when generating a blueprint by passing the
--pod
option, as described in #1619 (This PR is a cleaned up version of #1879)The pod option uses the original blueprint, but transforms the tokens used for the
mapFile
method. Previously this was limited to__name__
, which was used throughout the default blueprint files folder. There are two additional default tokens required for blueprints that support pod structure:__path__
and__test__
. These three tokens allow quite a bit of flexibility, including support for nested pod generation:The tokens are used to generate a fileMap based on the structure defined. If
--structure
is not passed, the default structure will be used. The__test__
token is necessary because the name token changes per structure:The fileMap (which is defined on the locals), is used for the
mapFile
method, which writes out all the paths for the files to be generated when the blueprint is installed. MapFile simply loops through the defined tokens and replaces the token instances in files with the defined values.This PR also introduces the ability to define custom tokens to be used in the files to be installed. This is achieved through a new blueprint hook called
fileMapTokens
. The hook will be processed along with the locals, and add or override any tokens the author wishes. To add a token, the author must add the hook method to their blueprint index.js, defining each token, and a function that resolves the token value. See route and component blueprints for an example.