[FOLIO-845] enforce code style during build and PR Created: 19/Sep/17  Updated: 12/Nov/18  Resolved: 08/Nov/17

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Bug Priority: P2
Reporter: Jakub Skoczen Assignee: John Malconian
Resolution: Done Votes: 0
Labels: ci, montreal, sprint23
Remaining Estimate: Not Specified
Time Spent: 5 hours, 30 minutes
Original estimate: Not Specified

Issue links:
Blocks
is blocked by STCOR-87 Multiple eslint errors on master branch Closed
is blocked by UIORG-34 Multiple eslint errors on master branch Closed
is blocked by UIREQ-19 Many eslint errors on master branch Closed
is blocked by UIU-249 Lint error on master branch Closed
Sprint:

 Description   

For JS the code style will be checked by importing new project: stripes-eslint that will include the code style. For Java we want to rely on the code style enforced by SQ (we need to verify if it's aligned with our style).



 Comments   
Comment by John Malconian [ 28/Sep/17 ]

Couple of comments about this.

1. So Sonarqube does a few things that eslint does not (AFAIK). Unit test code coverage is a big one as well as pretty dashboard that can be easily referred to. There probably is a lot of redundancy in using both, but I'm not sure. I'm wondering if we should run both, at least for a while, and see where we net out.

2. I think all of the UI projects that have a npm lint script defined have something like 'eslint || true'. This will always return a '0' exit status. So running 'npm run lint' will make it difficult for CI to catch failures. I can always just run 'eslint' directly, but then there is ambiguity about what files and directories should be checked.

Comment by Mike Taylor [ 28/Sep/17 ]

1. Please, no more arbitrary barriers. By all means use Sonaqube on server-side stuff if you want. We already have code-quality tools that we chose on the client side.

2. The || true is cosmetic, to avoid having ESLint spill its guts whenever there's a non-clean lint. We can lose it without too much pain.

Comment by John Malconian [ 28/Sep/17 ]

Mike Taylor I can go with that. I only request that we lose '|| true' and I will be good to go!

Comment by Mike Taylor [ 28/Sep/17 ]

By all means – feel free to rip out the || true wherever you see it.

Comment by John Malconian [ 04/Oct/17 ]

ui-requests, ui-organization, ui-users, and stripes-core report lint errors. stripes-react-hotkeys has no linting set up. All other relevant client-side modules are now "lint-enabled" in the CI. Will create separate jira issues for outstanding client-side repos.

Comment by Mike Taylor [ 04/Oct/17 ]

stripes-react-hotkeys is a special case, and we should make no changes to it. It's a fork of the regular react-hotkeys which we had to make because its maintainer ignores PRs. We made only the changes we absolutely needed, and otherwise want to keep it as close to the original as possible.

Comment by John Malconian [ 08/Nov/17 ]

Linting via ESLint is enabled across all relevant front-end projects.

Generated at Thu Feb 08 23:08:44 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.