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

Upgrade linters, language servers #381

Merged
merged 11 commits into from
Nov 5, 2020
Merged

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Oct 8, 2020

References

Generally referenced in a few places, some of the other open PRs have already applied the python changes. Hopefully we can get all these diffs in one PR and move forward on other things.

Code changes

  • updates eslint, typescript and various plugins
  • updates yaml-language-server, which hopefully works with schema now
  • clean refresh of yarn.lock
  • runs formatters, linters
  • adds all _*.d.ts to .gitignore

User-facing changes

N/A (aside from yaml working better)

Backwards-incompatible changes

N/A

Chores

  • linted
  • tested
  • documented
  • changelog entry

@krassowski
Copy link
Member

It seems that the typescript upgrade gives as a difficult time on:

@krassowski/jupyterlab-lsp:     TypeScript diagnostics (customize using `[jest-config].globals.ts-jest.diagnostics` option):
@krassowski/jupyterlab-lsp:     src/features/diagnostics/diagnostics.ts:268:3 - error TS2610: 'settings' is defined as an accessor in class 'CodeMirrorIntegration', but is overridden here in 'DiagnosticsCM' as an instance property.

@bollwyvl
Copy link
Collaborator Author

Yeah, ick, will check it out next chance i get.

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 3, 2020

Giving this another shot... passes locally...

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 3, 2020

ah, i didn't grok that it was failing during tests... will investigate...

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 4, 2020

Some progress here: found some upstream language server issues, and have made some headway on fixing the property re-definitions... jest still failing, though...

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 4, 2020

Really sorry this fell off for a sec... got distracted by... things.

Anyhow: helluva yak shave, but everything just ran locally (though pulled it a bit hot without linting)

  • the TS2610 "fix" just overloads the get for settings and lab_integration with the feature types
    • we could make the features generic over their settings schema, and pass that down in a few places
  • jest(-ts) was annoying, but works now
    • some paths were weird (e.g. file://dummy.py vs file:///dummy.py)...
      • we should capture that as constant someplace, i guess,
        • ... and probably not use / (it's /unit-test someplace, which is better)
  • yaml-language-server speaks schema now, so those tests can pass 🎉 (or fail, and provide insight)
    • ... but vscode dropped a hot version of the json language service breaking backcompat 👻
      • our pin, is... fine. for now. for testing purposes. but i actually really want that to work
        • ... so i have a pr open upstream, which might drop before i get this monster done
  • bash-language-server disabled their error highlighting by default...
    • ... i turned it back on in the spec...
      • ... but then decided it's just better to patch the env during test, rather than force the opinion
  • bumped all the notebook metadata to the hottest python 3.7

Here's hoping on CI finally going through... or at least getting to where it can tell me something broken on a non-linux platform.

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 4, 2020

so yeah: i kinda just don't care about jest on windows, i guess...

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 4, 2020

Aw

@bollwyvl bollwyvl closed this Nov 4, 2020
@bollwyvl bollwyvl reopened this Nov 4, 2020
@bollwyvl bollwyvl requested a review from krassowski November 5, 2020 00:59
@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 5, 2020

Huzzah, back to green.

decodeURI(uri) !== decodeURI(current_uri) &&
decodeURI(uri) !== '/' + decodeURI(current_uri)
) {
if (!uris_equal(uri, current_uri)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm very apprehensive of such changes, but let's try it as tests pass...

@krassowski krassowski merged commit fbc9d1c into jupyter-lsp:master Nov 5, 2020
@krassowski
Copy link
Member

Thank you!

@krassowski krassowski mentioned this pull request Nov 5, 2020
@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Nov 5, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants