Vote #80631
完了Use stylelint to avoid errors and enforce conventions in CSS files
0%
説明
In the last few months we had some cases when we added/fixed broken CSS code (ex: r18173, r18144, #32838). These types of errors can be easily avoid if we use a lint tool for CSS (as we started using Rubocop for Ruby).
My proposal is to add "Stylelint":https://stylelint.io/ which is a very well know library, actively maintained, with good community and integrated by many tools.
Attached is a patch that:
Adds @package.json@ and @yarn.lock[1]@ to request the library as development dependency. To install and run it, I propose to use Yarn.¶
Adds @.stylelintrc@ with the initial list of rules from "here":https://stylelint.io/user-guide/rules#possible-errors. For now, I propose to start only with the possible-errors rules.¶
Adds @.stylelintignore@ to ignore some files from linting (jquery-ui-*.ccs for now).¶
Updates the documentation¶
Beside this, if the overall idea is accepted, I have in plan to add the following:
- Add more rules to enforce some coding standards in CSS files
- Add a Javascript lint tool like "ESLint":https://eslint.org/
- Use package.json to handle the version of the current JS libraries (ex: jQuery, jQuery UI, Chart, Raphael, Tribute, etc)
- Give a shoot to integrate Webpack as assets pipeline (In Rails 6, is "default":https://github.com/rails/webpacker)
Any feedback is really appreciated.
fn1. I prefer to have @yarn.lock@ committed in the repository in order to enforce the same version between Redmine installations.
journals
--------------------------------------------------------------------------------
Running Stylelint over the existing CSS code from public folder, returns the following violations:
<pre>
root@df2fd1389bf4:/work# node_modules/.bin/stylelint "public/stylesheets/**/*.css"
public/stylesheets/application.css
561:1 ✖ Unexpected duplicate selector "div#issue-changesets div.changeset", first used at line 560 no-duplicate-selectors
682:77 ✖ Unexpected duplicate "white-space" declaration-block-no-duplicate-properties
1084:1 ✖ Unexpected duplicate selector "table.progress", first used at line 1075 no-duplicate-selectors
1560:1 ✖ Unexpected duplicate selector ".sort-handle", first used at line 706 no-duplicate-selectors
1619:83 ✖ Unexpected duplicate "color" declaration-block-no-duplicate-properties
public/stylesheets/responsive.css
137:5 ✖ Unexpected duplicate "height" declaration-block-no-duplicate-properties
264:18 ✖ Unexpected missing generic font family font-family-no-missing-generic-family-keyword
399:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
400:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
401:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
423:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
424:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
425:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
492:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
493:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
494:5 ✖ Unexpected duplicate "display" declaration-block-no-duplicate-properties
510:5 ✖ Unexpected duplicate "height" declaration-block-no-duplicate-properties
804:3 ✖ Unexpected duplicate selector "#issue_tree .issue > td:not(.checkbox), #relations .issue > td:not(.checkbox)", first used at line 787 no-duplicate-selectors
828:3 ✖ Unexpected duplicate selector "#issue_tree .issue > td.subject, #relations .issue > td.subject", first used at line 799 no-duplicate-selectors
public/stylesheets/rtl.css
322:3 ✖ Unexpected duplicate "margin-left" declaration-block-no-duplicate-properties
322:21 ✖ Unexpected duplicate "margin-right" declaration-block-no-duplicate-properties
377:14 ✖ Unexpected empty block block-no-empty
</pre>
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
Updated the patch to ignore consecutive duplicated properties with different values in order to not trigger a violation for the following case:
<pre><code class="css">
display: -webkit-flex;
display: -ms-flexbox;
display: -webkit-box;
display: flex;
</code></pre>
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
I think it is nice to have Stylelint. Setting the target version to 4.2.0.
--------------------------------------------------------------------------------
It's a good idea. And I'm completely with you Marius that we need some coding standards on the stylesheets.
--------------------------------------------------------------------------------
Marius, let me ask one thing.
Should I commit @yarn.lock@ file? I do not understand Yarn well, but it seems that it is not necessary to commit because it is generated automatically when @yarn install@ is executed. If I commit this file, I have to keep updating it in the future.
--------------------------------------------------------------------------------
Go MAEDA wrote:
> Marius, let me ask one thing.
>
> Should I commit @yarn.lock@ file? I do not understand Yarn well, but it seems that it is not necessary to commit because it is generated automatically when @yarn install@ is executed. If I commit this file, I have to keep updating it in the future.
Yes, it should be committed in the source control and updated when we will do changes to @packages.json@. It's a good practice to commit the lock file under source control, regardless the language/framework (Ruby, PHP, Javascript) in order to ensure the same version across all servers/installations/machines and avoid issues generated by new packages version (Redmine has a lot of issues generated by this case).
The @install@ command first checks for the lock file and if exists, it will install the same version of the packages. If doesn't exist, it will be generated based on which versions are defined in the @*.json@ and available at that moment.
https://classic.yarnpkg.com/en/docs/yarn-lock/#toc-check-into-source-control
In Redmine, from what I known, the Gemfile.lock is not committed because we support multiple environments and platforms, but for Javascript packages is not the case.
--------------------------------------------------------------------------------
Committed the patch. Thank you for improving the dev environment.
--------------------------------------------------------------------------------
related_issues
relates,Closed,32890,Fix violations reported by Stylelint
Admin Redmine さんが4年以上前に更新
- カテゴリ を Code cleanup/refactoring_30 にセット
- 対象バージョン を 4.2.0_152 にセット