feat(cli): use a custom repository base class #2235

gczobel-f5 · 2019-01-10T16:30:05Z

Allow the user to specify a custom Repository class to inherit from.
CLI supports custom repository name

via an interactive prompt
via CLI options
via JSON config

Two tests modified to use the new parameter to pass
Modified tests:

generates a kv repository from default model
generates a kv repository from decorator defined model

Checklist

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

raymondfeng · 2019-01-10T17:54:15Z

@gczobel-f5 Thank you for the patch. Please run npm run lint:fix to format the code per our settings.

raymondfeng · 2019-01-10T17:57:14Z

We usually don't merge commits from master into the feature branch. Instead, we rebase. For example:

git checkout my-branch
git pull --rebase upstream master
git push -f

gczobel-f5 · 2019-01-10T18:10:24Z

We usually don't merge commits from master into the feature branch. Instead, we rebase. For example:
git checkout my-branch
git pull --rebase upstream master
git push -f

Sorry, my first PR. I missed that part of the community guidelines.

raymondfeng · 2019-01-10T18:12:00Z

packages/cli/generators/repository/index.js

+    });
+
+    return _.first(artifactPaths);
+  }


This can be possibly simplified by using utils to map the artifact name to a file name instead of inferring the artifact name from the file names.

You mean using AST? Actually, all the CLIs use the same logic... take the first word of the filename as the artifact name...

No. I meant to say to use a function to infer the file name from the artifact name and do the match.

bajtos

This is a great feature 👍

IIUC the proposed implementation, it does not distinguish between repository classes that are already bound to a model (e.g. ProductRepository) and generic repository classes that are intended to be inherited from (e.g. MyCrudRepository). As a result, the CLI allows users to generate invalid project configuration, where a repository for one model (e.g. Category) is inheriting from a repository that's already bound to another model (e.g. ProductRepository).

Can we come up with a better solution? For example, we can use a different naming convention for generic repository classes (my-crud-repository.base.ts) or expect generic repositories to be defined in a different directory (e.g. repositories/base, repositories/generic, etc.).

One more scenario to consider: I expect that many base repository classes will be shared via an npm package. In the future, we should improve our CLI to discover base repository classes from the dependencies too and include them in the list of available repositories offered to the user.

packages/cli/test/fixtures/repository/index.js

bajtos · 2019-01-11T09:30:41Z

We usually don't merge commits from master into the feature branch. Instead, we rebase.

Sorry, my first PR. I missed that part of the community guidelines.

Maybe this information is not easy to find in our guidelines? The file DEVELOPING.md does not mention the rebase process at all. The only relevant information I could find is in the section How to rebase your branch on loopback.io.

@gczobel-f5 Any suggestions on how to improve our documentation for contributors to make it easier to find the information about the rebase process? Would you mind submitting a pull request to contribute those changes yourself?

gczobel-f5 · 2019-01-15T08:03:29Z

@gczobel-f5 Any suggestions on how to improve our documentation for contributors to make it easier to find the information about the rebase process? Would you mind submitting a pull request to contribute those changes yourself?

The page at code-contrib-lb4.html is very clear about all the process from the installation up to the commit guidelines... I think that after the commit guidelines need to be mentioned the rebase process or at least a link to the page where the rebase process is explained.

bajtos

Looks pretty good 👍

Please work with @raymondfeng to address his comments and get his approval too.

packages/cli/generators/repository/index.js

raymondfeng · 2019-01-24T22:47:32Z

@gczobel-f5 Any more improvements based on @bajtos's comments before we can land it?

gczobel-f5 · 2019-01-24T23:17:02Z

Hi, I'm traveling this week. I have more improvements to do but will be ready in middle of next week Regards

…

________________________________ From: Raymond Feng <[email protected]> Sent: Thursday, January 24, 2019 2:47:36 PM To: strongloop/loopback-next Cc: Gustavo Czobel; Mention Subject: Re: [strongloop/loopback-next] feat(cli): use a custom repository base class (#2235) EXTERNAL MAIL: [email protected] @gczobel-f5<https://github.com/gczobel-f5> Any more improvements based on @bajtos<https://github.com/bajtos>'s comments before we can land it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#2235 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ArHXMn2pGxf_N1uGOznRBuD8qizUEzVBks5vGjgIgaJpZM4Z522Y>.

gczobel-f5 · 2019-01-26T06:00:05Z

@gczobel-f5 Any more improvements based on @bajtos's comments before we can land it?

@raymondfeng Done. Sorry for the delay.

gczobel-f5 · 2019-02-03T07:37:14Z

@raymondfeng @bajtos changes are done from 26/Jan. Any reason this is not merged?

Allow the user to specify a custom Repository class to inherit from. CLI supports custom repository name * via an interactive prompt * via CLI options * via JSON config Two tests modified to use the new parameter to pass Modified tests: * generates a kv repository from default model * generates a kv repository from decorator defined model

bajtos · 2019-02-04T11:36:16Z

I have rebased the changes on top of the latest master and squashed all changes into a single commit. Let's wait for CI results before landing.

bajtos · 2019-02-04T12:02:56Z

Landed, thank you @gczobel-f5 for this great improvement and sorry for the delay.

gczobel-f5 requested review from bajtos and raymondfeng as code owners January 10, 2019 16:30

raymondfeng reviewed Jan 10, 2019

View reviewed changes

bajtos reviewed Jan 11, 2019

View reviewed changes

packages/cli/test/fixtures/repository/index.js Outdated Show resolved Hide resolved

bajtos assigned raymondfeng and bajtos Jan 11, 2019

bajtos added feature CLI community contribution labels Jan 11, 2019

bajtos approved these changes Jan 18, 2019

View reviewed changes

packages/cli/generators/repository/index.js Outdated Show resolved Hide resolved

packages/cli/generators/repository/index.js Outdated Show resolved Hide resolved

dhmlau mentioned this pull request Jan 24, 2019

[CLI] Use a custom Repository base class #2149

Closed

gczobel-f5 mentioned this pull request Jan 31, 2019

docs(cli): use a custom repository base class #2317

Merged

7 tasks

bajtos mentioned this pull request Feb 4, 2019

Best practice for encrypting model properties #2095

Open

3 tasks

bajtos merged commit edbbe88 into loopbackio:master Feb 4, 2019

nabdelgadir mentioned this pull request Mar 7, 2019

docs: add repository base class #2551

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): use a custom repository base class #2235

feat(cli): use a custom repository base class #2235

gczobel-f5 commented Jan 10, 2019 •

edited

Loading

raymondfeng commented Jan 10, 2019

raymondfeng commented Jan 10, 2019

gczobel-f5 commented Jan 10, 2019

raymondfeng Jan 10, 2019

gczobel-f5 Jan 10, 2019

raymondfeng Jan 10, 2019

gczobel-f5 Jan 15, 2019

bajtos left a comment

bajtos commented Jan 11, 2019

gczobel-f5 commented Jan 15, 2019

bajtos left a comment

raymondfeng commented Jan 24, 2019

gczobel-f5 commented Jan 24, 2019 via email •

edited

Loading

gczobel-f5 commented Jan 26, 2019

gczobel-f5 commented Feb 3, 2019

bajtos commented Feb 4, 2019

bajtos commented Feb 4, 2019

feat(cli): use a custom repository base class #2235

feat(cli): use a custom repository base class #2235

Conversation

gczobel-f5 commented Jan 10, 2019 • edited Loading

Checklist

raymondfeng commented Jan 10, 2019

raymondfeng commented Jan 10, 2019

gczobel-f5 commented Jan 10, 2019

raymondfeng Jan 10, 2019

Choose a reason for hiding this comment

gczobel-f5 Jan 10, 2019

Choose a reason for hiding this comment

raymondfeng Jan 10, 2019

Choose a reason for hiding this comment

gczobel-f5 Jan 15, 2019

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

bajtos commented Jan 11, 2019

gczobel-f5 commented Jan 15, 2019

bajtos left a comment

Choose a reason for hiding this comment

raymondfeng commented Jan 24, 2019

gczobel-f5 commented Jan 24, 2019 via email • edited Loading

gczobel-f5 commented Jan 26, 2019

gczobel-f5 commented Feb 3, 2019

bajtos commented Feb 4, 2019

bajtos commented Feb 4, 2019

gczobel-f5 commented Jan 10, 2019 •

edited

Loading

gczobel-f5 commented Jan 24, 2019 via email •

edited

Loading