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

update express-composition example's npm files and include the example in npm test #2552

Merged
merged 2 commits into from
Mar 8, 2019

Conversation

nabdelgadir
Copy link
Contributor

Removed files from package.json of express-composition example so the correct files download when using lb4 example express-composition. Then changed the express-composition example based on how it's done in #2316 so they're included when running npm test at the root of loopback-next.

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

@nabdelgadir nabdelgadir self-assigned this Mar 7, 2019
@emonddr emonddr self-requested a review March 7, 2019 20:17
async function changeDataSourceConfig() {
/**
* Override default config for DataSource for testing so we don't write
* test data to file when using the memory connector.
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered using the approach we are recommending in https://loopback.io/doc/en/lb4/Testing-your-application.html#create-a-test-datasource? Maybe it's not a good one, IDK.

It would be great to have a single prescribed way, used consistently both in our docs and the example repos. I think that's out of scope of this pull request though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following how it was done in the Todo example, but I can definitely look into that way and update the examples/the docs in a different PR.

@nabdelgadir nabdelgadir force-pushed the fix-express-npm-settings branch from 8a2c543 to f9d144b Compare March 8, 2019 14:51
@nabdelgadir nabdelgadir merged commit ed6f32c into master Mar 8, 2019
@nabdelgadir nabdelgadir deleted the fix-express-npm-settings branch March 8, 2019 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants