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

reduce number of top-level files #180

Merged
merged 3 commits into from
Jan 26, 2020
Merged

Conversation

bollwyvl
Copy link
Collaborator

References

Code changes

Moves a lot of things around. There is a hard reference to LANGSERVERS.md in the source: we should probably just inline that or something... or point to readthedocs when we do that.

User-facing changes

The repo is a tad cleaner on first blush:

  • fits on a full screen, meh
    • Screenshot from 2020-01-25 15-19-16

Backwards-incompatible changes

shouldn't create any issues on PRs...

Chores

  • linted
  • tested
  • documented
  • changelog entry

@bollwyvl
Copy link
Collaborator Author

To really go after it, we need all the meta junk from python:

  • setup.{cfg,py}
  • MANIFEST.in
    and js:
  • package.json
  • lerna.json
  • yarn.lock
  • .prettierignore
    .. in separate dirs altogether. I'm happy to do that (once I get at least this working), but that will start to impact the usability, i fear.

@bollwyvl
Copy link
Collaborator Author

Another thought: if the DX is indeed getting onerous, we can fix it Once And For All and introduce a top-level automation tool, and indeed, that might even be worthwhile, especially as we increase the number of built artifacts (#177, #163). If we want to go that way, this PR is a fine place to do it: i find it onerous to incrementally change automation if we can get consensus vision of where we want to go.

make would be great, but in 2020, it's still hard to reliably get a working windows toolchain, same goes for node-based stuff, so i'd favor doit. Others i've tried include pants and scons: again, they are rather heavy opinions.

Positing doit (but likely very similar for other tools): this would replace scripts with a single, top-level dodo.py, and after a pip install -e . (and getting node somehow), you'd type doit <thing>. Running without any args would basically run the entire azure pipeline locally, which is actually fantastic for reviewing PRs and CI. If you do a touch of extra work, it can do the make-like thing: e.g. if you change the ts, but don't touch the python, it won't even run the python unit tests, but would run the acceptance tests (after first rebuilding your lab so you don't get the thinger blocked by dialog warning).

Anyhow, food for thought...

@bollwyvl bollwyvl changed the title [wip] reduce number of top-level files reduce number of top-level files Jan 25, 2020
@bollwyvl
Copy link
Collaborator Author

Well, I guess I'll take it passing the first time. No doubt i missed some stuff in the markdown links, but that will be tested aggressively on #177, so less inclined to worry about it at this point...

package.json Show resolved Hide resolved
@@ -211,7 +211,7 @@ class LSPPopup extends VDomRenderer<LSPStatus.Model> {
Documentation:{' '}
<a
href={
'https://github.com/krassowski/jupyterlab-lsp/blob/master/LANGUAGESERVERS.md'
'https://github.com/krassowski/jupyterlab-lsp/blob/master/docs/LANGUAGESERVERS.md'
Copy link
Member

Choose a reason for hiding this comment

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

One lazy option would be to point to a version-specific commit and have a link to master in the document itself (something like "there might be an updated version of this document >click here<".)

@krassowski krassowski merged commit 87e08ea into jupyter-lsp:master Jan 26, 2020
@krassowski krassowski mentioned this pull request Jan 26, 2020
4 tasks
@bollwyvl bollwyvl mentioned this pull request Jan 26, 2020
7 tasks
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