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

lb4 repository,model,controller commands generate artifacts with 'lint' issues #4205

Closed
3 tasks
emonddr opened this issue Nov 25, 2019 · 5 comments · Fixed by #4431
Closed
3 tasks

lb4 repository,model,controller commands generate artifacts with 'lint' issues #4205

emonddr opened this issue Nov 25, 2019 · 5 comments · Fixed by #4431
Assignees
Labels
bug developer-experience Issues affecting ease of use and overall experience of LB users good first issue

Comments

@emonddr
Copy link
Contributor

emonddr commented Nov 25, 2019

I tried running : npm run test

after creating the artifacts from the Todo tutorial, and the linting after the tests fail.

I did a comparison of the generated files, and the files after npm run lint:fix and these are the differences I found:

Controller:

image

Repository:

image

Model:

image

Steps to reproduce

Current Behavior

Expected Behavior

Link to reproduction sandbox

Additional information

Related Issues

See Reporting Issues for more tips on writing good issues

Acceptance Criteria

  • Create the Todo application with the lb4 commands, and then ensure the npm run lint completes without error.
  • fix templates ( might need to update snapshots test as well)
  • add solutions to fix the lint error in docs
@emonddr emonddr added the bug label Nov 25, 2019
@raymondfeng
Copy link
Contributor

Use --format flag. See standard options at https://loopback.io/doc/en/lb4/Application-generator.html#options.

@bajtos bajtos added developer-experience Issues affecting ease of use and overall experience of LB users good first issue labels Nov 26, 2019
@bajtos
Copy link
Member

bajtos commented Nov 26, 2019

Use --format flag. See standard options at https://loopback.io/doc/en/lb4/Application-generator.html#options.

Personally, I consider that approach as a workaround. We should strive to emit code that passes our linter rules.

I think the issue pointed out by @emonddr should be easy to fix.

If we think --format is the solution that more people should use, then let's find a way how to enable that option by default. (But that's out of scope of this issue.)

@dhmlau
Copy link
Member

dhmlau commented Nov 26, 2019

@emonddr, could you please add the acceptance criteria? Thanks.

@agnes512 agnes512 added this to the Dec 2019 milestone Dec 3, 2019
@dhmlau dhmlau removed this from the Dec 2019 milestone Dec 13, 2019
@jannyHou jannyHou self-assigned this Dec 24, 2019
@jannyHou
Copy link
Contributor

I am on the latest cli @loopback/cli@1.27.0, the created todo example (lb4 example --> select todo) already fixes the lint errors in CONTROLLER and MODEL:

  • model:
    Screen Shot 2019-12-30 at 2 14 39 PM
  • controller:
    Screen Shot 2019-12-30 at 2 17 25 PM

The REPOSITORY has the correct code: changing

constructor(@inject('datasources.db') dataSource: juggler.DataSource) {
    super(Todo, dataSource);
  }

to

constructor(
  @inject('datasources.db') dataSource: juggler.DataSource
) {
    super(Todo, dataSource);
  }

results in lint error, so we should stay with the current behaviour.

There is a new lint error found in the controller file, it's already fixed in the latest master branch, so a new release will get rid of it:

Screen Shot 2019-12-30 at 2 13 32 PM

@jannyHou
Copy link
Contributor

I will wait for a new release before closing this story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug developer-experience Issues affecting ease of use and overall experience of LB users good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants