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

Split big index.js file into multiple smaller files #1761

Merged
merged 4 commits into from
Oct 9, 2023

Conversation

yuvipanda
Copy link
Collaborator

@yuvipanda yuvipanda commented Oct 3, 2023

Fixes #777

A screen recording of me testing this is available in
#1758

@choldgraf
Copy link
Member

omg is it finally happening?!

@consideRatio consideRatio reopened this Oct 9, 2023
@consideRatio consideRatio added maintenance Under the hood improvements and fixes code:html/js/css html/js/css changes. labels Oct 9, 2023
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Thank you for working this @yuvipanda!!!

LGTM except a few (I think) easy-to-resolve comments!

binderhub/static/js/src/repo.js Outdated Show resolved Hide resolved
binderhub/static/js/src/urls.js Outdated Show resolved Hide resolved
binderhub/static/js/index.js Show resolved Hide resolved
binderhub/static/js/src/repo.js Outdated Show resolved Hide resolved
- Follows the plan laid out in
jupyterhub#777
- Only non-trivial move is to cleanup the `_config` call to
  use the more modern 'fetch' API rather than the older
  XMLHttpRequest API

Fixes jupyterhub#777
Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
@yuvipanda
Copy link
Collaborator Author

Thanks @consideRatio. I've applied your suggested changes.

I think after this gets merged, I'll enable prettier for JS in this repo as well, so that should handle all other formatting inconsistencies.

@consideRatio consideRatio merged commit 79a15e2 into jupyterhub:main Oct 9, 2023
13 of 15 checks passed
@consideRatio
Copy link
Member

Wieeee thank you @yuvipanda for taking the time to this!!! ❤️ 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code:html/js/css html/js/css changes. maintenance Under the hood improvements and fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor index.js megafile into mutliple files
3 participants