[#30196] - [4.0] Removing Hound and moving to Drone
- Fixed in Code Base
- 29 Jul 2020
- Medium
- Build: 4.0-dev
- # 30196
- Diff
- Hackwar:j4stylelint
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
For Javascript and SCSS codestyle, we are using Hound to check against our codestyle standards. Due to unknown reasons, Hound failed to catch several violations to our standards, since it only reports on files that have been modified instead of the whole codebase each time. So we would like to have a full check on all our SCSS and JS files each time.
Coincidentally we already do JS codestyle checks on each build/PR in drone, so we can simply drop the whole JS codestyle stuff from Hound.
Regarding SCSS a solution might be to simply call scss-lint in Drone and be done with it, but that brings up another issue: scss-lint is Ruby-based and is not really maintained anymore. SCSS at one point was Ruby-based, but has since been rewritten in NodeJS and thus a Ruby-based linter adds yet another environment for the developer to add just for this. Long story short: scss-lint is not the prefered tool here anymore.
The solution seems to be stylelint, which can be simply installed during setup of the project with npm i. However, now we have to adopt the configuration from scss-lint to stylelint. The following abstract of the configuration file is still missing in the stylelint configuration. Maybe we drop some of these, maybe we enforce others. That would be up for discussion here.
linters:
BorderZero:
enabled: true
convention: zero # or `none`
exclude:
- _normalize.scss
Comment:
enabled: true
exclude:
- _normalize.scss
- bootstrap.scss
style: silent
ElsePlacement:
enabled: true
style: same_line # or 'new_line'
IdSelector:
enabled: true
ImportPath:
enabled: true
leading_underscore: false
filename_extension: false
NameFormat:
enabled: true
allow_leading_underscore: true
convention: hyphenated_lowercase # or 'camel_case', or 'snake_case', or a regex pattern
PseudoElement:
enabled: true
SpaceAroundOperator:
enabled: true
style: one_space # or 'at_least_one_space', or 'no_space'
UrlFormat:
enabled: true
I would be very happy if someone could support me in fixing the codestyle violations and to finish the stylelint configuration.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Unit Tests |
| Labels |
Added:
?
?
|
||
| Labels |
Added:
?
Removed: ? |
||
| Category | Unit Tests | ⇒ | Unit Tests NPM Change |
| Labels |
Added:
NPM Resource Changed
?
Removed: ? |
||
| Category | Unit Tests NPM Change | ⇒ | Unit Tests Administration com_media NPM Change Templates (admin) Repository |
| Labels |
Added:
?
Removed: ? |
||
| Category | Unit Tests NPM Change Administration com_media Templates (admin) Repository | ⇒ | Unit Tests Administration com_media NPM Change Templates (admin) JavaScript Repository |
| Labels |
Added:
?
Removed: ? |
||
| Labels |
Added:
?
Removed: ? |
||
Can you re-run Drone please. It's showing results for a previous commit
Personally I would extend stylelint's standard config rather than using your own rules. Meaning that if any new standards come into play in the future, you don't need to manually add them, but instead simply update the dependency.
You can then built on top of it.
Just run a test locally with stylelint. The only errors being returned now are:
max-nesting-depthselector-max-compound-selectors
I don't want to really dive into Atum's SCSS to be honest.
Thank you for your feedback. Right now I just wanted to move the infrastructure over, I'm open to changing rules afterwards. Since you seem to have been the one to introduce this, I was hoping that you could elaborate a bit on why you chose these rules and if those left are necessary from your point of view or if we could drop them. It sounds a bit like you took a default style and maybe modified it a bit. Unless you have any objections, I would simply run with what is in this PR now, fix the last errors and then be done with it...
I originally copied over the rules used by Bootstrap when they were using the Ruby scss-lint gem. Can't quite remember if I made any tweaks as it was a few years ago now, but you can change the rules to whatever you think is best for Joomla ;)
Thank you. I will then keep it as it is and later migrate to another, established codestyle...
| Labels |
Added:
?
Removed: ? |
||
You can also remove Gemfile and Gemfile.lock from the root
rdeutz
- | Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-07-29 10:54:43 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
Removed: ? |
||
@C-Lodder since you initially setup the scss-lint configuration, can you give some feedback here? Maybe also help with the violations which came up?