-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor: move example packages to examples folder #1231
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
Conversation
virkt25
left a comment
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.
Thank you for the PR! I've taken a quick look, will have a more thorough look later today / tomorrow. A few minor changes are needed.
.gitignore
Outdated
| packages/*/package | ||
| packages/_sandbox | ||
| .sandbox | ||
| examples/*/*.tgz |
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 wonder if **/*.tgz works (and same for below) instead of having the same thing repeat for packages and examples.
.prettierignore
Outdated
| packages/*/dist | ||
| packages/*/api-docs | ||
| packages/cli/generators/*/templates | ||
| examples/*/dist |
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.
Does **/*/dist work?
CODEOWNERS
Outdated
| packages/rest/* @bajtos @raymondfeng | ||
| packages/testlab/* @bajtos | ||
| examples/todo/* @bajtos @virkt25 | ||
| examples/example-hello-world/* @b-admike |
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.
The folder name should be updated to examples/hello-world
| { | ||
| "$schema": "http://json.schemastore.org/tsconfig", | ||
| "extends": "../build/config/tsconfig.common.json", | ||
| "extends": "../../packages/build/config/tsconfig.common.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.
Let's change this to ./node_modules/@loopback/build/config/tsconfig.common.json
shimks
left a comment
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 is a great PR! I've pointed out one small point that has to do with consistency, but everything else looks great!
.gitignore
Outdated
| packages/*/package | ||
| packages/_sandbox | ||
| .sandbox | ||
| examples/*/dist* |
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.
Let's be consistent and use **/dist* as well. Same thing with examples/*/package
shimks
left a comment
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 for the contribution. I would wait on a review from @miroslav or @raymondfeng before merging
virkt25
left a comment
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.
One last change (and a rebase) is needed and I think this should be good to land! Thanks for your effort :)
.prettierignore
Outdated
| packages/*/dist | ||
| packages/*/api-docs | ||
| packages/cli/generators/*/templates | ||
| **/*/dist |
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 think just a **/dist should work.
the packages folder currently contains all the examples, the example packages don't contain any
loopback logic, therefore it makes sense to move the examples to a separate folder. All example
packages are thus moved from `packages/example-{name}` to `examples/{name}`.
|
Merging as it's a trivial PR but a big one and don't want to have it require another rebase. |
|
@jwooning Thanks for your contribution!! Your PR has landed 🎉 |
The packages folder currently contains all the examples, the example packages don't contain any
loopback logic, therefore it makes sense to move the examples to a separate folder. All example
packages are thus moved from
packages/example-{name}toexamples/{name}.Note that not all tests succeed, because the cli examples generator tries to fetch the examples from the changed examples structure, which will fail because the master is not yet restructured.
Fix #1218
Checklist
npm testpasses on your machinepackages/cliwere updatedpackages/example-*were updated