-
Notifications
You must be signed in to change notification settings - Fork 273
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
Monorepo #313
Monorepo #313
Conversation
Update del to the latest version 🚀
Export `NodePlopAPI` interface for plop#147
Update eslint to the latest version 🚀
# Conflicts: # .gitignore
OK, I think we're just about ready to merge this! @amwmedia @evelynhathaway can you take a look at this and let me know if there's anything obvious missing here and what you think of the whole process? Namely, a few things:
@amwmedia one question in particular for you:
|
packages/plop/package.json
Outdated
"interpret": "^2.2.0", | ||
"liftoff": "^4.0.0", | ||
"minimist": "^1.2.5", | ||
"node-plop": "*", |
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 will 100% break when we release a, say, v2 of node-plop
.
We either need to lock this to:
"node-plop": "*", | |
"node-plop": "~0.30.0", |
Or, if we decide to bump to 1.0.0
, like I'm hoping we can:
"node-plop": "*", | |
"node-plop": "^1.0.0", |
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.
Agreed. Also, caret works for both so we can use it to select nonbreaking changes either way.
https://docs.npmjs.com/cli/v6/using-npm/semver#caret-ranges-123-025-004
This is the main source code for the `plop` package. | ||
|
||
For documentation, please refer to [our website](https://plopjs.com/) or [our main README at the root of the repository](../../README.md). |
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.
Up for suggestions about how to communicate this better. This should only ever be seen by contributors on their machine or on GitHub.com.
Otherwise, our prepublishOnly.js
script should replace this with the README.md
that matches our existing README.md
.
The reason for two README.md files is so that we can still display a proper documented README for newcomers to the repo, while still providing a README for contributors pointing them to the right place
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.
To avoid the need for the switch back and forth, could a git hook that syncs these help?
Or commit to having two copies of some of the information and work on making the main readme talk about all of the projects? I kinda like having a separation between the monorepo readme and the published ones. A markdown comment reminding folks to make their changes in two locations.
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'm actually not a huge fan of having two different READMEs. The contributors will forget, we'll forget, then we have to manually squash things, it's kinda a mess IMO.
While I usually like having a separate monorepo README and published one, I think it makes sense here. Open for alternative READMEs after this, but I think this is probably the right move for now
// Used by eslint for linting | ||
{ | ||
"extends": "./packages/node-plop/types/tsconfig.json", | ||
} |
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 see where ESLint is using this.
Do you use either of these?
https://github.com/typescript-eslint/tslint-to-eslint-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.
Yeah, we use typescript-eslint
for TS files itself
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 you point to where it's used? My original comment was that I don't think it is currently being used.
packages/plop/package.json
Outdated
"interpret": "^2.2.0", | ||
"liftoff": "^4.0.0", | ||
"minimist": "^1.2.5", | ||
"node-plop": "*", |
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.
Agreed. Also, caret works for both so we can use it to select nonbreaking changes either way.
https://docs.npmjs.com/cli/v6/using-npm/semver#caret-ranges-123-025-004
This is the main source code for the `plop` package. | ||
|
||
For documentation, please refer to [our website](https://plopjs.com/) or [our main README at the root of the repository](../../README.md). |
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.
To avoid the need for the switch back and forth, could a git hook that syncs these help?
Or commit to having two copies of some of the information and work on making the main readme talk about all of the projects? I kinda like having a separation between the monorepo readme and the published ones. A markdown comment reminding folks to make their changes in two locations.
For the plopjs.com site generator. You can see the file that does the work here. It's essentially pulling the MD file from the repo and replacing the top part of the file with some front-matter for the static page generator. So long as that file didn't move (which I don't think it did), the site generation should be fine. |
This PR merges in all of
node-plop
(and preserves the history for archival purposes) and makes this repository contain 100% of the code required forplop
to run with two packages present.Closes plopjs/node-plop#211
Checklist:
package.json
links to reflect directory structure (so that when someone clicks on "repo" it goes to the subrepoCommit message enforcementPost-merge checklist:
node-plop
repo to avoid duplicate issues in the futurenode-plop
issues to this reponode-plop
PRsplop
andnode-plop
to update NPM website links