[#39382] - css lint not running on drone ?
- Closed
- 23 Dec 2022
- Medium
- Build: 4.2-dev
- # 39382
This is NOT a comment on the PR for dark mode
That PR (#39366) has exposed an issue with drone
lint:css is supposed to check the codestyling in the scss files
Steps to reproduce the issue
on a local developer environment run the following
npm run lint:css
Expected result
PS C:\htdocs\j42> npm run lint:css
> joomla@4.0.0 lint:css
> stylelint --config build/.stylelintrc.json "administrator/components/com_media/resources/**/*.scss" "administrator/templates/**/*.scss" "build/media_source/**/*.scss" "templates/**/*.scss" "installation/template/**/*.scss"
PS C:\htdocs\j42>
In other words no errors or warnings
Actual result
.... truncated becuase this issue is NOT about the errors in that PR it is that drone is not reporting those errors and it should be
36:5 ✖ Expected "background-color" to come before "border-bottom" order/properties-order
79:9 ✖ Expected "color" to come before "background" order/properties-order
2090 problems (2090 errors, 0 warnings)
Additional comments
So something is not right with drone - it definitely used to work as we made several PR in the past to fix lint errors. As it stands right now that PR could have been merged with over 2000 errors
cc @Hackwar @rdeutz @dgrammatiko tagging you as I am sure you know more about the drone tasks than me
joomla-cms-bot
- | Labels |
Added:
No Code Attached Yet
|
||
You also might want to check the projects' .stylelintrc.json. It contains a very strict set of rules that will most likely need updating when:
- The
stylelintdependency is updated. - A CSS feature reaches stage 3 or 4.
Also might be worth fixing the Node.js versions that Drone has been tasked with using. For the dependency installation, it's using 16-bullseye-slim, but for linting it's using node:current-alpine. Perhaps the -slim variation of the package is causing the issue? Could also try the full image.
In drone task are executed sequentially by default and all tasks after failing tasks are skipped. Looking at https://ci.joomla.org/joomla/joomla-cms/59978/1/ phpcs is failing and that task is before scss-cs task witch is skipped because of that.
My only concern right now is that the test isnt running - thanks for the explanation @wojsmol that kind of makes sense except that when I look at another example that is not the case https://ci.joomla.org/joomla/joomla-cms/59919
@brianteeman keep in mind that the tests are running in parallel so in one case the linting process might finish before some other task that throws and in another the throw comes before the lint task even started. Confusing but that's always the case with asynchronous code...
@dgrammatiko Drone can depend on another task, e.g wait till it's finished. So whether any of the single tasks are written asynchronously, it shouldn't matter
@dgrammatiko Drone can
dependon another task, e.g wait till it's finished. So whether any of the single tasks are written asynchronously, it shouldn't matter
Oh, that aligns with @wojsmol 's comment but I still think that some processes are done in parallel. But maybe the linters should be always allowed to finish (unless clone or composer failed then bail). As I said I'm not very familiar with drone, I'm using the GitHub actions...
Before my first comment I was checking drone configuration file in 4.2-dev branch in 4.3-dev we have few tasks with depends_on and here is documentation for drone parallelism.
I don't understand the issue right now. So far I haven't seen a drone build which had a scss-cs build which reported back successfull even though it found lots of issues. All examples shown here either ran successfully or aborted the build before ariving at the scss-cs step. And when I execute lint:css locally, it doesn't report any errors for our current 4.3-dev branch. Am I missing something or are you just assuming drone didn't report errors because drone aborted after the first failed step for the dark mode PR, since the PHP codestyle already is not correct.
Simply said: For the SCSS codestyle to be checked, the PHP stuff has to work and be in our codestyle first.
Simply said: For the SCSS codestyle to be checked, the PHP stuff has to work and be in our codestyle first.
ok - now I understand. Not obvious at all from drone that this is the case.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-12-23 22:49:00 |
| Closed_By | ⇒ | brianteeman |
We "optimised" drone to fail fast. Most PRs first have PHP codestyle issues and it would be a waste of our resources to run all tests to the end when the codestyle at the beginning already fails.
Node scripts correctly throw so it should be a drone thing and I'm not educated enough to make any guesses, sorry...