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

Context Type for wasm-pack #123

Closed
mgattozzi opened this issue May 9, 2018 · 4 comments
Closed

Context Type for wasm-pack #123

mgattozzi opened this issue May 9, 2018 · 4 comments
Labels
enhancement New feature or request question Further information is requested refactor

Comments

@mgattozzi
Copy link
Contributor

@data-pup came up with an interesting PoC in https://github.com/ashleygwilliams/wasm-pack/pull/122. I definitely think we should consider it as a possibility going forward, but I felt the change was a bit large and drastic to do all at once and wanted to defer it to a discussion first.

We essentially would thread a Context type to determine decisions for the program. This isn't unprecedented. For instance cargo and rustc use this concept as well. I also happen to use this at work extensively and this can help reduce the need for multiple extra function parameters.

I think this would be a good idea but I would love to hear thoughts because:

  1. This is a big change in how we maintain and think about wasm-pack
  2. We have a lot of other pressing things to get done first in terms of triage
  3. If we did want to do this what would our upgrade path be? How can we make this work in multiple PRs rather than one big one so we don't let anything slip through the cracks?

CC/ @ashleygwilliams as this is a really big change and I'm not sure how this fits into your vision of wasm-pack and the code base in general

@mgattozzi mgattozzi added enhancement New feature or request question Further information is requested refactor labels May 9, 2018
@ashleygwilliams
Copy link
Member

in general i think this pattern is a good one that we should aim for. need to think more on how we can do it iteratively tho... 🤔 does anyone else have any thoughts? i definitely would want to do this only after #18 because at the moment i dont super trust our tests :(

@mgattozzi
Copy link
Contributor Author

What do you mean by iteratively? I have some thoughts on how to do this but I'm not sure what you mean here

@data-pup
Copy link
Member

data-pup commented Jun 5, 2018

It's been a little while since I did this, but I remember the init command being the sub-command that caused the most trouble. We could start by refactoring the pack and publish commands first, before thinking more about the init command.

Also agree with you that this should come after #18

@ashleygwilliams
Copy link
Member

this is very out of date and the code base has changed a lot since then. i'm not against this type of refactor, but let's propose it again in a new issue with the newer codebase in mind!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested refactor
Projects
None yet
Development

No branches or pull requests

3 participants