8000 pre-commit CI is failing · Issue #834 · pyscript/pyscript · GitHub
[go: up one dir, main page]
Skip to content

pre-commit CI is failing #834

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

Jump to bottom
Closed
3 tasks done
JeffersGlass opened this issue Oct 6, 2022 · 5 comments · Fixed by #871 or #882
Closed
3 tasks done

pre-commit CI is failing #834

JeffersGlass opened this issue Oct 6, 2022 · 5 comments · Fixed by #871 or #882
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release type: bug Something isn't working

Comments

@JeffersGlass
Copy link
Contributor
JeffersGlass commented Oct 6, 2022 •
edited
Loading

Checklist

  • I added a descriptive title
  • I searched for other issues and couldn't find a solution or duplication
  • I already searched in Google and didn't find any good information or help

What happened?

Starting with commit c75f885, the pre-commit CI check is failing for all commits and PRs. Neither eslint nor tsc seem to show those errors when building locally, for either me nor @antocuni (as of a few days ago).

What browsers are you seeing the problem on? (if applicable)

No response

Console info

No response

Additional Context

No response
@JeffersGlass JeffersGlass added type: bug Something isn't working needs-triage Issue needs triage and removed needs-triage Issue needs triage labels Oct 6, 2022
@tedpatrick
Copy link
Contributor

ESLINT ==> Strict
TypeScript ==> Not Strict

This is the source of the ESLint warnings. In strict, Typescript shows 200+ warnings. Once things settle a bit, we will address in a PR.

@JeffersGlass
Copy link
Contributor Author

I think there must be something about how the pre-commit CI action works that I don't understand. eslint warnings shouldn't fail the build step - we've had a few hundred warnings for months and months, symptom of a rapidly evolving codebase.. Now it seems like in GitHub CI it's actually erroring out...

But running pre-commit run --all-files locally has lots of warnings locally, but no errors. Isn't the CI pipeline using the same hooks that would get executing with pre-commit run? There must be something I'm missing.

@madhur-tandon
Copy link
Member

Is it possible to SSH into the runners to investigate? It could be done with circleCI, but I am not sure with Github Actions.

@tedpatrick
Copy link
Contributor

I do not think anything is wrong with CICD or ESLint, this is highlighting that we lack TypeScript types in many areas. If we fix this root cause, everything improves.

@antocuni
Copy link
Contributor
antocuni commented Oct 7, 2022

For reference, I reported it upstream:
pre-commit/pre-commit#2537

The author seems to think that it's a problem of my local environment, although I don't think it's the case. Do you confirm that pre-commit run --all-files passes on all your machines?

I still think it's a bug of pre-commit: if it passes locally, it should passes on CI. It it fails on CI, it should fail locally.

This is the source of the ESLint warnings.

yes, but this is only part of the problem. ESLint used to pass and then it started to fail suddenly out of the blue

@marimeireles marimeireles added the backlog issue has been triaged but has not been earmarked for any upcoming release label Oct 11, 2022
@JeffersGlass JeffersGlass mentioned this issue Oct 19, 2022
Merged
@FabioRosado FabioRosado mentioned this issue Oct 22, 2022
Merged
@tedpatrick tedpatrick moved this to Closed in PyScript OSS Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backlog issue has been triaged but has not been earmarked for any upcoming release type: bug Something isn't working
Projects
PyScript OSS
Archived in project
Milestone
No milestone
5 participants
@antocuni @JeffersGlass @marimeireles @madhur-tandon @tedpatrick
0