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

Remove unused webpack.ts #96

Closed
wants to merge 1 commit into from
Closed

Remove unused webpack.ts #96

wants to merge 1 commit into from

Conversation

ndmlny-qs
Copy link
Collaborator

src/webpack.ts is not used, and is removed in this commit.

The following files

  • index.ts
  • loader.ts
  • manager.ts

have been modified to remove commented out code, and fixes a few != and == where they should be !== and ===.

`src/webpack.ts` is not used, and is removed in this commit.

The following files

- `index.ts`
- `loader.ts`
- `manager.ts`

have been modified to remove commented out code, and fixes a few `!=`
and `==` where they should be `!==` and `===`.
@ndmlny-qs ndmlny-qs requested a review from mattpap May 1, 2023 19:01
@ndmlny-qs ndmlny-qs self-assigned this May 1, 2023
@mattpap
Copy link
Contributor

mattpap commented May 2, 2023

This code actually seems to be doing something.

Before this PR:
image

and after:
image

The warnings in the before image are the doing of my PR #95.

@ndmlny-qs
Copy link
Collaborator Author

thanks for showing this does something. do you know why it is using these fonts? I don't fully understand why a wraper would need fonts, but I'll close it out

@ndmlny-qs ndmlny-qs closed this May 2, 2023
@mattpap
Copy link
Contributor

mattpap commented May 2, 2023

I think fonts are used for icons in ipyvolume's toolbar, which is currently broken regardless (it's actually sort of present in Firefox). I would be happy to get rid of this code, but I suppose there will be another mechanism necessary to load resources like fonts.

@ndmlny-qs ndmlny-qs deleted the remove-webpack-module branch June 20, 2023 18:19
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