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

feat: add defineCrudRepositoryClass #3867

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

hacksparrow
Copy link
Contributor

@hacksparrow 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 self-assigned this Oct 4, 2019
@bajtos
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 Compare October 4, 2019 13:09
@hacksparrow
Copy link
Contributor Author

@bajtos PTAL, updated.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

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

packages/rest-crud/src/repository-builder.ts Show resolved Hide resolved
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from f36784d to 32e6de5 Compare October 4, 2019 13:36
packages/rest-crud/README.md Outdated Show resolved Hide resolved
packages/rest-crud/src/repository-builder.ts Show resolved Hide resolved
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 32e6de5 to 13749fe Compare October 4, 2019 14:15
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 13749fe to 0347afc Compare October 30, 2019 09:56
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.

👍

@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch 3 times, most recently from e15cdb0 to 52c3020 Compare October 31, 2019 08:50
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 Compare October 31, 2019 14:41
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 Compare October 31, 2019 15:32
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);
Copy link
Member

Choose a reason for hiding this comment

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

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 Compare November 1, 2019 09:35
Add `defineCrudRepositoryClass` - a helper to create named repository classes
@hacksparrow hacksparrow force-pushed the feat/defineCrudRepositoryClass branch from 38b902d to 091d8c4 Compare November 1, 2019 09:39
@hacksparrow hacksparrow merged commit 8e3e21d into master Nov 1, 2019
@hacksparrow hacksparrow deleted the feat/defineCrudRepositoryClass branch November 1, 2019 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants