Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 40 commits into from
Sep 18, 2024
Merged

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

Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
24a30ab
Add Stylelint and Prettier configuration for CSS stylesheets.
junhaoliao Sep 10, 2024
653e71b
Update lint scripts to include CSS checks and fixes.
junhaoliao Sep 10, 2024
c1e9aa6
Add GitHub action problem matcher for Stylelint.
junhaoliao Sep 10, 2024
64a751c
Fix file-matcher remove path.
junhaoliao Sep 10, 2024
c95c20c
Remove path from `remove-matcher`.
junhaoliao Sep 10, 2024
308c319
Move `continue-on-error` to job meta to allow the job to be marked as…
junhaoliao Sep 10, 2024
8cd6e29
Add back `continue-on-error` on "npm --prefix new-log-viewer/ run lin…
junhaoliao Sep 10, 2024
b71d699
typo: Add back `continue-on-error` on "npm --prefix new-log-viewer/ r…
junhaoliao Sep 10, 2024
b09f16e
Remove `continue-on-error` and enhance conditional lint checks.
junhaoliao Sep 10, 2024
12a8bab
Always run "ataylorme/eslint-annotate-action@v3".
junhaoliao Sep 10, 2024
2c3c242
Add working directory for "Register Stylelint problem matcher".
junhaoliao Sep 10, 2024
676e929
Move working directory to `run: "npm run lint:css-check"`.
junhaoliao Sep 10, 2024
0a18614
Fix path in "Register Stylelint problem matcher"
junhaoliao Sep 10, 2024
e14fd6e
Wrap names with quotes.
junhaoliao Sep 10, 2024
78a859a
Remove unused npm script `lint:ci`.
junhaoliao Sep 10, 2024
764ab4f
Use sed to rename new-log-viewer paths.
junhaoliao Sep 10, 2024
d07d9b3
Replace single quote with double for sed.
junhaoliao Sep 10, 2024
c1ab8fd
Do sed on stderr as well.
junhaoliao Sep 10, 2024
7706e45
Fix lint step to correctly exit on error.
junhaoliao Sep 10, 2024
c7b50fc
Allow match if there's no leading space?
junhaoliao Sep 10, 2024
3de697f
Mark line numbers mandatory.
junhaoliao Sep 10, 2024
bb9307d
Correct indices.
junhaoliao Sep 10, 2024
ccc8a47
Fix regexp.
junhaoliao Sep 10, 2024
8f420b4
Remove color from stylelint output.
junhaoliao Sep 10, 2024
bafc21b
Make leading white spaces optional.
junhaoliao Sep 11, 2024
dae94ae
change pattern
junhaoliao Sep 11, 2024
4268be5
add severity.
junhaoliao Sep 11, 2024
94387dc
use github formatter instead
junhaoliao Sep 11, 2024
bf6b072
Lint stylesheets.
junhaoliao Sep 11, 2024
64b465a
Remove unused problem matcher.
junhaoliao Sep 11, 2024
64e7197
Add names to lint step actions.
junhaoliao Sep 11, 2024
009f24d
Add back npm script "npm run lint:check".
junhaoliao Sep 11, 2024
5481538
Merge branch 'main' into lint-css
junhaoliao Sep 11, 2024
0c6fa4f
Merge branch 'main' into lint-css
junhaoliao Sep 16, 2024
52beffc
Reformat CSS files.
junhaoliao Sep 16, 2024
b6db9a1
Merge branch 'y-scope:main' into lint-css
junhaoliao Sep 16, 2024
dbd2e87
Simplify lint scripts - Apply suggestions from code review
junhaoliao Sep 18, 2024
c597457
Update lint script configurations.
junhaoliao Sep 18, 2024
c0d4b71
Simplify lint workflow.
junhaoliao Sep 18, 2024
fff19ae
Update lint script and GitHub workflow for CI.
junhaoliao Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions new-log-viewer/.prettierrc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
tabWidth: 4
useTabs: false
singleQuote: false
quoteProps: consistent
printWidth: 100
endOfLine: lf
7 changes: 7 additions & 0 deletions new-log-viewer/.stylelintrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": [
"stylelint-config-standard",
"stylelint-config-clean-order/error",
"stylelint-prettier/recommended"
]
}
1,402 changes: 1,163 additions & 239 deletions new-log-viewer/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

15 changes: 12 additions & 3 deletions new-log-viewer/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
"main": "src/index.tsx",
"scripts": {
"build": "webpack --config webpack.prod.js",
"start": "webpack serve --open --config webpack.dev.js",

"lint": "npm run lint:check",
"lint:check": "eslint src webpack.*.js --max-warnings 0",
"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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@junhaoliao junhaoliao Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. 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.
  2. 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.
  3. 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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\""

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you would need to modify workflow to use lint:ci

"lint:check:js": "eslint src webpack.*.js --max-warnings 0",
"lint:fix": "npm-run-all --parallel --continue-on-error \"lint:check:* -- --fix\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay ltgm

},
"repository": {
"type": "git",
Expand Down Expand Up @@ -49,8 +52,14 @@
"html-webpack-plugin": "^5.6.0",
"mini-css-extract-plugin": "^2.9.0",
"monaco-editor-webpack-plugin": "^7.1.0",
"npm-run-all": "^4.1.5",
"prettier": "^3.3.3",
"react-refresh": "^0.14.2",
"style-loader": "^4.0.0",
"stylelint": "^16.9.0",
"stylelint-config-clean-order": "^6.1.0",
"stylelint-config-standard": "^36.0.1",
"stylelint-prettier": "^5.0.2",
"typescript": "^5.6.2",
"webpack": "^5.92.0",
"webpack-cli": "^5.1.4",
Expand Down
20 changes: 13 additions & 7 deletions new-log-viewer/src/components/DropFileContainer/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,27 @@

.hover-mask {
position: absolute;
z-index: var(--ylv-drop-file-container-hover-mask-z-index);
top: 0;
width: 100%;
height: 100%;
background-color: rgba(2, 88, 168, 0.2);

display: flex;
align-items: center;
justify-content: center;
z-index: var(--ylv-drop-file-container-hover-mask-z-index);

width: 100%;
height: 100%;

background-color: rgb(2 88 168 / 20%);
}

.hover-message {
z-index: var(--ylv-drop-file-container-hover-message-z-index);

padding: 8px;
color: #616161;
font-size: 0.875rem;

font-family: var(--ylv-ui-font-family), sans-serif;
font-size: 0.875rem;
color: #616161;

background-color: #f3f3f3;
z-index: var(--ylv-drop-file-container-hover-message-z-index);
}
2 changes: 1 addition & 1 deletion new-log-viewer/src/components/MenuBar/PageNumInput.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.page-num-input input::-webkit-outer-spin-button,
.page-num-input input::-webkit-inner-spin-button {
-webkit-appearance: none;
margin: 0;
appearance: none;
}

.page-num-input-num-pages-text {
Expand Down
2 changes: 1 addition & 1 deletion new-log-viewer/src/components/MenuBar/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.menu-bar {
display: flex;
flex-direction: row;
height: var(--ylv-status-bar-height);
align-items: center;
height: var(--ylv-status-bar-height);
}
8 changes: 5 additions & 3 deletions new-log-viewer/src/components/StatusBar/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
.status-bar {
align-items: center;
position: absolute;
bottom: 0;

display: flex;
position: absolute;
align-items: center;

width: 100%;
}

.status-message {
padding-left: 8px;
flex-grow: 1;
padding-left: 8px;
}
4 changes: 2 additions & 2 deletions new-log-viewer/src/components/modals/SettingsModal/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.settings-dialog-title {
display: flex;
justify-content: space-between;
align-items: center;
flex-wrap: wrap;
align-items: center;
justify-content: space-between;
}

.settings-dialog-title-text {
Expand Down
12 changes: 7 additions & 5 deletions new-log-viewer/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
html, body, #root {
margin: 0;
height: 100%;
html,
body,
#root {
width: 100%;
height: 100%;
margin: 0;
}

html {
Expand All @@ -10,8 +12,8 @@ html {

:root {
/* font-family globals */
--ylv-ui-font-family: -apple-system, BlinkMacSystemFont, system-ui, Ubuntu, "Droid Sans",
Roboto;
--ylv-ui-font-family: -apple-system, "BlinkMacSystemFont", system-ui, "Ubuntu", "Droid Sans",
"Roboto";

/* size globals */
--ylv-status-bar-height: 32px;
Expand Down
Loading