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

train works in a web-worker #17

Merged
merged 4 commits into from
Mar 31, 2024
Merged

train works in a web-worker #17

merged 4 commits into from
Mar 31, 2024

Conversation

AlexErrant
Copy link
Member

As of Vite v5.1.6, wasm-bindgen-rayon works in a web-worker.

FYI @ishiko732

Related: #11 RReverser/wasm-bindgen-rayon#4 (comment)

@AlexErrant AlexErrant merged commit 3a2e342 into main Mar 31, 2024
1 check passed
@AlexErrant AlexErrant deleted the demo-worker branch March 31, 2024 16:18
@L-M-Sherlock
Copy link
Member

It's better to use squash merging because the commit history will be more concise.

@AlexErrant
Copy link
Member Author

AlexErrant commented Apr 1, 2024

I personally prefer a more verbose history as I believe it makes the git history more readable. Each commit does exactly one thing, and if it takes several commits to build a feature that's fine.

So like, the history of this PR is simple/easy to read: update packages, revert a commit, fix some issues, then remove a package.

This history is important as it allows me to do things like revert that commit in step 2. That reverted commit was part of this PR, and I would not have been able to revert if it was squashed into one commit. Also, during the investigation phase of this PR, I was able to check out an intermediate commit for the other PR, update Vite, notice that web-workers now work, and therefore determine that an upstream issue was fixed. Squashing history for conciseness prevents all this.

To be clear, I'm not saying all git repositories should work this way - this is just my preferred way of working with git :)

@L-M-Sherlock
Copy link
Member

L-M-Sherlock commented Apr 1, 2024

Fine. What about rebase merging? It could also keep the verbose history of commits.

@AlexErrant
Copy link
Member Author

Hmm I do directly commit to main frequently. My primary objective is a readable git history, and merges (I feel) are an important part of that. If a feature can be done in one commit, then great, I could be convinced to rebase it into main. If not, then having them grouped together into a PR/merge commit has two bonuses; I can revert an entire feature with one commit if necessary, and I can revert parts of a feature as I did above. In the most recent PR from Ishiko, for example, with my guiding light being a readable git history, they added a config, and then later I added some comments justifying why that config is there. In that case the git narrative is 1) config added 2) why config was added. I'm also generally leery of rebase merges because they rewrite history, and I feel like history is important. I actually rebase all the time in my own private branches - it's how I build a "readable" history - but I rarely rebase when merging. I see no reason not to do it for single-commit PRs though! Again, this is all just my personal workflow - I'm not saying anyone else should work this way!

@L-M-Sherlock
Copy link
Member

OK. It's totally understandable. I just gave advice in my stand. I very appreciate your contribution. Just let everything continue as usual.

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