loopback-next icon indicating copy to clipboard operation
loopback-next copied to clipboard

fix: add missing configs

Open aaqilniz opened this issue 1 year ago • 4 comments

Here is the reason why I've made these changes.

We have three ways to pass options to a generator.

  1. the default values
  2. Passing individual argument
  3. passing through the configuration object (json object)

I will take the discoverer as an example.

It has an option views with the default value of true. The generator fails to assign the value of the views from the config object as in the base-generator, the condition would always be true. The object this.options will have the views set to true (the default). This is the line if (!this.options[o]) {.

Please note that this happens only when a generator has an option with the default value being truthy (views in discoverer & server option in openapi generator)

Checklist

  • [x] DCO (Developer Certificate of Origin) signed in all commits
  • [x] npm test passes on your machine
  • [x] New tests added or existing tests modified to cover all changes
  • [x] 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 👈

aaqilniz avatar Nov 11 '23 16:11 aaqilniz

Pull Request Test Coverage Report for Build 7781694227

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on fix/configs at 55.415%

Totals Coverage Status
Change from base Build 7776209974: 55.4%
Covered Lines: 9568
Relevant Lines: 12457

💛 - Coveralls

coveralls avatar Nov 11 '23 16:11 coveralls

@aaqilniz, is this PR ready for review? The changes LGTM.

dhmlau avatar Dec 15 '23 17:12 dhmlau

Hi, @dhmlau. The PR is finally ready.

aaqilniz avatar Jan 28 '24 08:01 aaqilniz

Hi, @samarpanB. Can you please have a look at the PR?

aaqilniz avatar Jul 26 '24 07:07 aaqilniz