Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: update todo-list example #4412

Merged
merged 3 commits into from Jan 23, 2020
Merged

docs: update todo-list example #4412

merged 3 commits into from Jan 23, 2020

Conversation

@nabdelgadir
Copy link
Contributor

nabdelgadir commented Jan 13, 2020

Update the todo-list docs to match the example's code.

Closes #4161 and closes #2050.

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@nabdelgadir nabdelgadir self-assigned this Jan 13, 2020
@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch 2 times, most recently from 91bd544 to 15bcaac Jan 14, 2020
@nabdelgadir nabdelgadir marked this pull request as ready for review Jan 14, 2020
@nabdelgadir nabdelgadir requested review from bajtos and raymondfeng as code owners Jan 14, 2020
@nabdelgadir nabdelgadir requested a review from emonddr Jan 14, 2020
@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch from 15bcaac to 6f6fe6f Jan 14, 2020
@emonddr

This comment has been minimized.

Copy link
Contributor

emonddr commented Jan 16, 2020

Going through the updated doc right now. Not sure why we 'deselect' Enable docker, and leave all the others checked. Any reason?

@dhmlau

This comment has been minimized.

Copy link
Member

dhmlau commented Jan 16, 2020

Going through the updated doc right now. Not sure why we 'deselect' Enable docker, and leave all the others checked. Any reason?

That's a good question. I think you're referring to the todo tutorial: https://loopback.io/doc/en/lb4/todo-tutorial-scaffolding.html#create-your-app-scaffolding, right?
I think we should recommend users to accept all the defaults.

Thoughts? @strongloop/loopback-maintainers

@nabdelgadir

This comment has been minimized.

Copy link
Contributor Author

nabdelgadir commented Jan 17, 2020

Thanks for adding that commit @emonddr!

That's a good question. I think you're referring to the todo tutorial: https://loopback.io/doc/en/lb4/todo-tutorial-scaffolding.html#create-your-app-scaffolding, right?
I think we should recommend users to accept all the defaults.

@dhmlau @emonddr I think the reason was because the option says it adds docker files to the application and there aren't any docker files in the example. So either we can add the files, or keep the option disabled. I'm fine with adding them though.

@dhmlau

This comment has been minimized.

Copy link
Member

dhmlau commented Jan 17, 2020

Personally, I'd prefer to keep all the defaults in the "features" prompt and make the necessary changes in the example app.

@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch 2 times, most recently from dac8d26 to c9d6537 Jan 20, 2020
@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch from c9d6537 to 5f55210 Jan 20, 2020
@nabdelgadir nabdelgadir requested review from emonddr, agnes512 and jannyHou Jan 20, 2020
Copy link
Contributor

jannyHou left a comment

馃憦 great effort!

Copy link
Contributor

agnes512 left a comment

Nice job :D
Just have one concern about hasOne & belongsTo. The rest are just some nitpicks.

@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch from 5f55210 to af62fc3 Jan 21, 2020
Copy link
Contributor

agnes512 left a comment

馃憦

@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch 3 times, most recently from 658c889 to 6c3c0e0 Jan 22, 2020
@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch from 6c3c0e0 to 0e45a08 Jan 23, 2020
@nabdelgadir nabdelgadir merged commit a02b814 into master Jan 23, 2020
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.001%) to 92.382%
Details
@nabdelgadir nabdelgadir deleted the update-todolist-example branch Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.