[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: |
|
||||||||||||||||||||
| 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. |