Run typecheck and eslint on ./run lint
#6314
Conversation
|
not sure whether i'm missing something (since a 3 line change is a really small change) but it seems to work Output from new checks |
somebody1234
added
the
CI: No changelog needed
|
seems to run correctly on ci (fails as expected). i think i'll merge the eslint fixes and verify that it's working, then add a type error to verify that typecheck lint works |
somebody1234
requested review from
MichaelMauderer,
wdanilo and
kazcw
as code owners
|
errors on eslint error: ✔️ |
|
@somebody1234 so QA passed 🟢 how about these lint errors for linux https://github.com/enso-org/enso/actions/runs/4739322557/jobs/8414041628?pr=6314 we can fix them now?
|
|
@PabloBuchu as mentioned in the original comment, #6267 will fix the lint errors. not sure if lint failing blocks merge |
It blocks the merge; this is the main point of integrating this with |
|
ah, of course... was on mobile at the time so i couldn't tell whether it was required - but the other PR should be just about ready, so i think it should be fine to merge the other one, then this one - when the other is confirmed to be ready, of course |
Makes total sense! |
build/cli/src/lib.rs
Outdated
|
|
||
| let js_modules_root = ctx.repo_root.join("app/ide-desktop"); | ||
|
|
||
| Npm.cmd()?.current_dir(&js_modules_root).args(["run", "typecheck"]).run_ok().await?; | ||
|
|
||
| Npx.cmd()?.current_dir(&js_modules_root).args(["eslint", "."]).run_ok().await?; |
There was a problem hiding this comment.
Why are we creating these newlines here? I don't think they are needed - they just make the code longer.
There was a problem hiding this comment.
should be fixed now
|
confirmed to exit with success because the other PR has been merged in. there are 4 eslint warnings (not errors) for (two of the warnings should be fixed by another PR, the other two are for features that are not implemented yet) |
|
If this PR is waiting for other PRs, let's change it to draft. If it is ready to merge - lets merge it :) |
|
@wdanilo some CIs failing, not sure why + waiting for approval from a code owner |
|
lint check is successful though so it should be safe enough to merge |
|
@PabloBuchu @indiv0 would one of you mind reviewing? a small QA would be nice but i don't think it's strictly necessary as lint CI is only run on linux. i've merged develop into this branch locally to test (will not actually merge in, just checking there will no issues after merging into develop) and will mark as ready for merge if it runs correctly |
|
oh, actually not sure why some js fixes snuck into this PR, but they're already here anyway. anyway, lint run has succeeded locally |
|
note: the "Engine CI / Engine (linux)" is required but failing with output: |
|
As we need this PR and the fails are engine-related only, merging it now. |
* develop: Fix cut-off in text visualisations (#6421) Infer correct synthetic name for nested modules (#6525) Delete unused websocket dependency (#6535) Run typecheck and eslint on `./run lint` (#6314) Force pending saves if client closes abruptly (#6514) Continued Execution Context work and some little fixes (#6506) IDE's logging to a file (#6478) Fix application config (#6513) Cloud/desktop mode switcher (#6448) Fix doubled named arguments bug (#6422) Reimplement `enso_project` as a proper builtin (#6352) Fix layer ordering between dropdown and breadcrumbs backgrounds. (#6483) Multiflavor layers (#6477) DataflowAnalysis preserves dependencies order (#6493) Implement `create_database_table` for Database Table (#6467) Limit Dead Letter logging (#6482) More reliable shutdown of the EnsoContext to save resources (#6468) Make execution mode `live` default for CLI (#6496)
…-5075 * develop: Build nightly 3 hours earlier. (#6551) Cache result of slow function resolution on Any that is present on a hot path (#6536) Fix cut-off in text visualisations (#6421) Infer correct synthetic name for nested modules (#6525) Delete unused websocket dependency (#6535) Run typecheck and eslint on `./run lint` (#6314) Force pending saves if client closes abruptly (#6514)
This reverts commit 7885145.
Pull Request Description
Fixes #6244
Important Notes
The lint check will fail because there are eslint errors. #6267 should fix this.The typecheck check should be tested by making some code invalid. the simplest way to do this would be to change a variable name.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.