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

JS: Remove all references to global URL constants #1781

Merged
merged 2 commits into from
Oct 23, 2023

Conversation

yuvipanda
Copy link
Collaborator

Instead they are passed in wherever needed. Cleaner code, and path towards everything being more testable

Ref #774

@yuvipanda yuvipanda added maintenance Under the hood improvements and fixes code:html/js/css html/js/css changes. labels Oct 20, 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.

@yuvipanda this LGTM and could be merged at this point, but since I pushed a commit I figure I'll let you merge after reviewing it.

yuvipanda and others added 2 commits October 23, 2023 15:25
Instead they are passed in wherever needed. Cleaner code,
and path towards everything being more testable
@yuvipanda
Copy link
Collaborator Author

Thanks for the catch and the fix, @GeorgianaElena and @consideRatio! I've pulled, rebased and tested to make sure it works ok. I'll merge this now and deploy to mybinder.org shortly.

I'm going to hopefully add optional typechecking (no typescript) to help catch errors like this.

@yuvipanda yuvipanda merged commit c1c5af6 into jupyterhub:main Oct 23, 2023
13 checks passed
consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Oct 23, 2023
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.

3 participants