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’ll 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 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 Compare January 14, 2020 02:54
@nabdelgadir nabdelgadir marked this pull request as ready for review January 14, 2020 03:33
@nabdelgadir nabdelgadir requested a review from emonddr January 14, 2020 03:33
@emonddr
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
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
Copy link
Contributor Author

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
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 Compare January 20, 2020 04:30
Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 great effort!

Copy link
Contributor

@agnes512 agnes512 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Compare January 21, 2020 13:57
Copy link
Contributor

@agnes512 agnes512 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch 3 times, most recently from 658c889 to 6c3c0e0 Compare January 22, 2020 16:30
@nabdelgadir nabdelgadir force-pushed the update-todolist-example branch from 6c3c0e0 to 0e45a08 Compare January 23, 2020 15:12
@nabdelgadir nabdelgadir merged commit a02b814 into master Jan 23, 2020
@nabdelgadir nabdelgadir deleted the update-todolist-example branch January 23, 2020 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants