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

feat: add defineCrudRepositoryClass #3867

Merged
merged 1 commit into from Nov 1, 2019

Conversation

@hacksparrow
Copy link
Member

hacksparrow commented Oct 4, 2019

Add defineCrudRepositoryClass - a helper to create named repository classes.

Addresses #3733.

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 馃憟

@hacksparrow hacksparrow requested review from bajtos and raymondfeng as code owners Oct 4, 2019
@hacksparrow hacksparrow self-assigned this Oct 4, 2019
@bajtos

This comment has been minimized.

Copy link
Member

bajtos commented Oct 4, 2019

@hacksparrow I am confused. The pull request says "Add defineCrudRepositoryClass - a helper to create named repository classes", but then it changes 24 files and introduces also a booter class. Can you please check?

@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch 2 times, most recently from c498c64 to f36784d Oct 4, 2019
@hacksparrow

This comment has been minimized.

Copy link
Member Author

hacksparrow commented Oct 4, 2019

@bajtos PTAL, updated.

@emonddr
emonddr approved these changes Oct 4, 2019
Copy link
Member

bajtos left a comment

The code changes look reasonable. Please add a new section to README to show how to use this new API.

@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from f36784d to 32e6de5 Oct 4, 2019
packages/rest-crud/README.md Outdated Show resolved Hide resolved
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 32e6de5 to 13749fe Oct 4, 2019
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 13749fe to 0347afc Oct 30, 2019
Copy link
Contributor

jannyHou left a comment

馃憤

@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch 3 times, most recently from e15cdb0 to 52c3020 Oct 31, 2019
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/README.md Outdated Show resolved Hide resolved
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch 2 times, most recently from b2d9b60 to bc42658 Oct 31, 2019
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/README.md Outdated Show resolved Hide resolved
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch 3 times, most recently from 0686f2c to fce609b Oct 31, 2019
packages/rest-crud/README.md Outdated Show resolved Hide resolved
```ts
const db = new juggler.DataSource({connector: 'memory'});
const ProductRepository = defineCrudRepositoryClass(Product);
const repo = new ProductRepository(db);

This comment has been minimized.

Copy link
@bajtos

bajtos Nov 1, 2019

Member

I find it confusing that the Controller section is showing DI + app.controller, but the Repository section is showing manual approach, which also does not take into account different lifetimes of a datasource (one instance for entire app life) vs. a repository (new repository created for each request).

@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from fce609b to 38b902d Nov 1, 2019
@bajtos
bajtos approved these changes Nov 1, 2019
Add `defineCrudRepositoryClass` - a helper to create named repository classes
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 38b902d to 091d8c4 Nov 1, 2019
@hacksparrow hacksparrow merged commit 8e3e21d into master Nov 1, 2019
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.005%) to 92.103%
Details
@hacksparrow hacksparrow deleted the feat/defineCrudRepositoryClass branch Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.