lint: Add support for CSS stylesheets in linting workflow. #66

junhaoliao · 2024-09-11T02:28:06Z

References

new-log-viewer series: #45 #46 #48 #51 #52 #53 #54 #55 #56 #59 #60 #61 #62 #63

Description

Add Stylelint config.
Add Prettier config and integrate with Stylelint.
Add npm script for linting.
Set up CSS linting in the Github lint workflow.

Validation performed

Created test branch for testing PR submitting to main: junhaoliao#6

With only CSS lint errors, only CSS lint errors got marked and the job has a failed status: https://github.com/junhaoliao/yscope-log-viewer/actions/runs/10803848528
When the lint errors are fixed, the job passed: https://github.com/junhaoliao/yscope-log-viewer/actions/runs/10803951760
With only ESLint lint errors, only ESLint errors got marked and the job has a failed status: https://github.com/junhaoliao/yscope-log-viewer/actions/runs/10803988196

… failed although the steps are continue on error.

…t:css-check".

…un lint:css-check".

new-log-viewer/package.json

davemarco · 2024-09-18T00:57:56Z

new-log-viewer/package.json

+    "lint:fix": "npm-run-all --sequential --continue-on-error lint:css-fix lint:js-fix",
+
+    "lint:css-check": "stylelint src/**/*.css",
+    "lint:css-ci": "npm run lint:css-check -- --formatter github",


I did some internet sloothing and found they plan to remove completely [1] [2]
) and recommend to just use third party extension. I made that change in my suggestion in package.json. It also removed deprecation warning

davemarco · 2024-09-18T01:01:23Z

.github/workflows/lint.yaml

@@ -29,4 +29,6 @@ jobs:
        with:
          node-version: 22
      - run: "npm --prefix new-log-viewer/ clean-install"
+      - run: "npm --prefix new-log-viewer/ run lint:css-ci"


Maybe just update this now to call lint:check:js and lint:check:css so lint:check dosen't run lint:css twice

junhaoliao · 2024-09-18T04:29:31Z

.github/workflows/lint.yaml

@@ -29,4 +29,6 @@ jobs:
        with:
          node-version: 22
      - run: "npm --prefix new-log-viewer/ clean-install"
+      - run: "npm --prefix new-log-viewer/ run lint:css-ci"
      - run: "npm --prefix new-log-viewer/ run lint:check"


Sorry I think this was supposed to be lint:js-check.

Suggested change

- run: "npm --prefix new-log-viewer/ run lint:check"

- run: "npm --prefix new-log-viewer/ run lint:js-check"

I think you rid of this later anyways. so i'm ignoring

yeah it turn out that no changes are needed in the workflow file after the scripts are simplified

Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>

davemarco · 2024-09-18T14:51:22Z

new-log-viewer/package.json

+    "lint:check": "npm-run-all --sequential --continue-on-error lint:check:*",
+    "lint:check:css": "stylelint src/**/*.css --formatter github",
+    "lint:check:js": "eslint src webpack.*.js --max-warnings 0",
+    "lint:fix": "npm-run-all --parallel --continue-on-error \"lint:check:* -- --fix\""


I guess the parallel might be slightly more convenient, but maybe could cause issues if the linters touch the same files? If they don't, im okay with it. If they do, maybe just use sequential to be safer

the linters touch the same files

Supposedly no at the moment. ESLint would only look into js, jsx, ts, and tsx files and we're asking Stylelint to look only into css files.

davemarco · 2024-09-18T14:53:20Z

.github/workflows/lint.yaml

@@ -29,4 +29,6 @@ jobs:
        with:
          node-version: 22
      - run: "npm --prefix new-log-viewer/ clean-install"
+      - run: "npm --prefix new-log-viewer/ run lint:css-ci"
      - run: "npm --prefix new-log-viewer/ run lint:check"


I think you rid of this later anyways. so i'm ignoring

davemarco · 2024-09-18T14:54:25Z

new-log-viewer/package.json

-    "lint:fix": "npm run lint:check -- --fix",
-    "start": "webpack serve --open --config webpack.dev.js"
+    "lint:check": "npm-run-all --sequential --continue-on-error lint:check:*",
+    "lint:check:css": "stylelint src/**/*.css --formatter github",


I'm happy to do this or just use the string formatter... I dont mind looking at logs

Sure. I think we can add the problem matcher for the string formatter in another PR soon.

Before I make the changes let's take a look at the approaches we can take:

Use a custom formatter in Stylelint and a custom action in the GitHub workflow.
PROS: We can view the errors in a nicely formatted way and we can have the errors annotated inline by GitHub.
CONS: setting up custom stuff takes time; we might not want to use custom stuff given they might not always be compatible with the tooling upgrades / GitHub workflow, which means we might not want to maintain it as a long term solution.

Use the default string formatter.
PROS: It's the default formatter which means it should have the best support from the tooling contributors.
CONS: We lose the ability to have the errors annotated inline (until we move the files into the project root). It outputs relative paths, but GitHub workflow problem matchers only resolves paths from the project root. Supposedly we can check in the problem matcher, which currently does nothing, in the same PR to save another PR, but it's hard to verify if things would actually work. If we spot any issues after we move the log-viewer files, we might still need to fix in the PR we move the files or another PR.

Use the github formatter.
PROS: We don't need any custom plugin to get the GitHub annotation working.
CONS: We will see a deprecated warning in the workflow results. We're using a deprecated but not yet removed formatter. If we don't upgrade the Stylelint version the formatter should still be there, and it's less likely we would upgrade it before we move the new-log-viewer files and migrate to the problem matcher approach.

I still think Approach 3 gives us the most benefits, but all CONs are minor since we might always need another PR to improve the lint experience after we move the new-log-viewer files.

Okay i think it dosen't matter so much as we are going to move soon anyways. I am okay with 3.
Do you like this by any chance? This way we still preserve normal formatting when running locally (i.e. run lint:check locally and lint:ci for workflow)

"lint:ci": "npm-run-all --sequential --continue-on-error lint:check:js \"lint:check:css -- --formatter github\""

you would need to modify workflow to use lint:ci

davemarco

Im good! Do you like for title "lint: Add support for CSS stylesheets in linting workflow."

junhaoliao added 29 commits September 10, 2024 17:12

Add Stylelint and Prettier configuration for CSS stylesheets.

24a30ab

Update lint scripts to include CSS checks and fixes.

653e71b

Add GitHub action problem matcher for Stylelint.

c1e9aa6

Fix file-matcher remove path.

64a751c

Remove path from remove-matcher.

c95c20c

Move continue-on-error to job meta to allow the job to be marked as…

308c319

… failed although the steps are continue on error.

Add back continue-on-error on "npm --prefix new-log-viewer/ run lin…

8cd6e29

…t:css-check".

typo: Add back continue-on-error on "npm --prefix new-log-viewer/ r…

b71d699

…un lint:css-check".

Remove continue-on-error and enhance conditional lint checks.

b09f16e

Always run "ataylorme/eslint-annotate-action@v3".

12a8bab

Add working directory for "Register Stylelint problem matcher".

2c3c242

Move working directory to run: "npm run lint:css-check".

676e929

Fix path in "Register Stylelint problem matcher"

0a18614

Wrap names with quotes.

e14fd6e

Remove unused npm script lint:ci.

78a859a

Use sed to rename new-log-viewer paths.

764ab4f

Replace single quote with double for sed.

d07d9b3

Do sed on stderr as well.

c1ab8fd

Fix lint step to correctly exit on error.

7706e45

Allow match if there's no leading space?

c7b50fc

Mark line numbers mandatory.

3de697f

Correct indices.

bb9307d

Fix regexp.

ccc8a47

Remove color from stylelint output.

8f420b4

Make leading white spaces optional.

bafc21b

change pattern

dae94ae

add severity.

4268be5

use github formatter instead

94387dc

Lint stylesheets.

bf6b072

junhaoliao marked this pull request as ready for review September 11, 2024 02:39

davemarco reviewed Sep 18, 2024

View reviewed changes

junhaoliao commented Sep 18, 2024

View reviewed changes

junhaoliao and others added 3 commits September 18, 2024 02:42

Simplify lint scripts - Apply suggestions from code review

dbd2e87

Co-authored-by: davemarco <83603688+davemarco@users.noreply.github.com>

Update lint script configurations.

c597457

Simplify lint workflow.

c0d4b71

junhaoliao requested a review from davemarco September 18, 2024 06:58

davemarco reviewed Sep 18, 2024

View reviewed changes

Update lint script and GitHub workflow for CI.

fff19ae

junhaoliao requested a review from davemarco September 18, 2024 19:09

davemarco approved these changes Sep 18, 2024

View reviewed changes

junhaoliao changed the title ~~lint: Configure Stylelint and Prettier for CSS stylesheets and set up related GitHub workflow.~~ lint: Add support for CSS stylesheets in linting workflow. Sep 18, 2024

junhaoliao merged commit 9440832 into y-scope:main Sep 18, 2024
1 check passed

davemarco mentioned this pull request Sep 25, 2024

new-log-viewer: Add event cursor, refactor state context, and redesign load page methods to support discontinuous log event numbers in preparation for log level filtering. #76

Merged

This was referenced Sep 26, 2024

new-log-viewer: Integrate latest clp-ffi-js with support for log-level filtering; Add log-level selector to StatusBar. #77

Merged

new-log-viewer: Replace fallback font with Inter. #78

Merged

davemarco mentioned this pull request Oct 4, 2024

new-log-viewer: Finish support for filtering by log-level and associated refactoring. #89

Merged

Henry8192 mentioned this pull request Oct 7, 2024

new-log-viewer: Add log query support in StateContextProvider. #80

Merged

This was referenced Oct 22, 2024

new-log-viewer: Fix filename wrapping and overflow in menu bar for filenames with hyphens on narrow windows (fixes #99). #100

Merged

Replace existing codebase with new-log-viewer and update docs. #101

Merged

junhaoliao mentioned this pull request Nov 4, 2024

Deprecate github formatter usage in script lint:ci. #110

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lint: Add support for CSS stylesheets in linting workflow. #66

lint: Add support for CSS stylesheets in linting workflow. #66

junhaoliao commented Sep 11, 2024 •

edited

Loading

davemarco Sep 18, 2024

davemarco Sep 18, 2024

junhaoliao Sep 18, 2024

davemarco Sep 18, 2024

junhaoliao Sep 18, 2024

davemarco Sep 18, 2024

junhaoliao Sep 18, 2024

davemarco Sep 18, 2024

davemarco Sep 18, 2024

davemarco Sep 18, 2024

junhaoliao Sep 18, 2024 •

edited

Loading

junhaoliao Sep 18, 2024

davemarco Sep 18, 2024

davemarco Sep 18, 2024

davemarco left a comment •

edited

Loading

	- run: "npm --prefix new-log-viewer/ run lint:check"
	- run: "npm --prefix new-log-viewer/ run lint:js-check"

lint: Add support for CSS stylesheets in linting workflow. #66

lint: Add support for CSS stylesheets in linting workflow. #66

Conversation

junhaoliao commented Sep 11, 2024 • edited Loading

References

Description

Validation performed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junhaoliao Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davemarco left a comment • edited Loading

Choose a reason for hiding this comment

junhaoliao commented Sep 11, 2024 •

edited

Loading

junhaoliao Sep 18, 2024 •

edited

Loading

davemarco left a comment •

edited

Loading