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.

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

Fix linting errors #915

Merged
merged 9 commits into from
Jul 31, 2024
Merged

Fix linting errors #915

merged 9 commits into from
Jul 31, 2024

Conversation

dhogan8
Copy link
Contributor

@dhogan8 dhogan8 commented Jul 30, 2024

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Jul 30, 2024

Deploying blog-site with  Cloudflare Pages  Cloudflare Pages

Latest commit: b164d44
Status: ✅  Deploy successful!
Preview URL: https://01f1b8b9.blog-site-602.pages.dev
Branch Preview URL: https://dallas-fix-linting-errors.blog-site-602.pages.dev

View logs

@dhogan8 dhogan8 force-pushed the dallas/fix-linting-errors branch 12 times, most recently from 9bd8867 to b566d9d Compare July 30, 2024 16:53
@dhogan8 dhogan8 changed the title Dallas/fix linting errors Fix linting errors Jul 30, 2024
@dhogan8 dhogan8 force-pushed the dallas/fix-linting-errors branch 3 times, most recently from 16016fe to b566d9d Compare July 30, 2024 17:01
@@ -0,0 +1,45 @@
name: Precious
Copy link
Member

Choose a reason for hiding this comment

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

We probably don't need a Precious github action since we already have a linting github action, which lints the whole repo; linting the whole is preferred in CI since things like node module updates could catch linting errors that a git diff list would miss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't clear to me why the linting.yml file didn't fail with all of the errors that were fixed in this PR though.

Copy link
Contributor

Choose a reason for hiding this comment

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

The beauty of precious is that we can run all sorts of linters and tidiers. For the blog, I would think adding something like typos would be helpful. I guess we could still run that via the node scripts, but that gets cumbersome really quickly.

Copy link
Member

Choose a reason for hiding this comment

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

It wasn't clear to me why the linting.yml file didn't fail with all of the errors that were fixed in this PR though.

It's because warnings don't return a non-zero exit code.

To fix this in stylelint, change defaultSeverity to error.

For eslint, add the --max-warnings=0 cli argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding something for typos does seem ideal for something like the blog site.

Thanks, I've removed the precious github action in b164d44 if it is preferred to use only the linting github action.

.browserslistrc Show resolved Hide resolved
#!/usr/bin/env bash

# This is for installing precious and other 3rd party libs needed for linting
# in CI
Copy link
Member

Choose a reason for hiding this comment

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

I guess this script would also need to be run at least once in dev environments, yeah?


You should also install our pre-commit hook. You can do this from your checkout
by running `git/setup.sh`. These hooks do things like ensure that the code you
commit is tidy and passes various linter checks.
Copy link
Member

Choose a reason for hiding this comment

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

This should also mention that precious needs to be installed and how to install it. For instance, I sometimes work on the blog site locally on my computer which currently doesn't have precious installed.

@dhogan8 dhogan8 requested a review from kevcenteno July 31, 2024 18:18
@dhogan8 dhogan8 merged commit 1ada0ee into main Jul 31, 2024
9 checks passed
@dhogan8 dhogan8 deleted the dallas/fix-linting-errors branch July 31, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants