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

Update parallel raytracing .gitignore #3606

Merged
merged 6 commits into from
Sep 27, 2023

Conversation

jthemphill
Copy link
Contributor

Running the build script creates some *.d.ts files and a snippets folder. Adding these to the .gitignore.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Couldn't we use the --no-typescript flag instead?
snippets would still be required though.

examples/raytrace-parallel/.gitignore Outdated Show resolved Hide resolved
@Liamolucko
Copy link
Collaborator

I'm not really sure why raytrace-parallel (and wasm-audio-worklet) puts its output into the current directory in the first place. All the other (non-Webpack-based) examples put their output into pkg instead: changing raytrace-parallel to do to the same would get rid of the need to even have this .gitignore, since pkg is already ignored by examples/.gitignore.

@jthemphill
Copy link
Contributor Author

All the other (non-Webpack-based) examples put their output into pkg instead: changing raytrace-parallel to do to the same would get rid of the need to even have this .gitignore, since pkg is already ignored by examples/.gitignore.

Sure! But since this now involves changing build.sh or the future build.py, I'm going to rebase this atop #3603 to avoid skew.

examples/raytrace-parallel/.gitignore Outdated Show resolved Hide resolved
examples/raytrace-parallel/index.html Outdated Show resolved Hide resolved
@jthemphill jthemphill force-pushed the raytracing-gitignore branch 2 times, most recently from e1b21c9 to 44496d0 Compare September 13, 2023 17:22
@daxpedda
Copy link
Collaborator

This is LGTM, just waiting for us to figure out if we can remove the .gitignore entirely.
See #3606 (comment).

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Sep 14, 2023
@daxpedda daxpedda self-assigned this Sep 14, 2023
@daxpedda daxpedda removed the waiting for author Waiting for author to respond label Sep 27, 2023
Copy link
Collaborator

@daxpedda daxpedda 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!

@daxpedda daxpedda merged commit 792be2d into rustwasm:main Sep 27, 2023
25 checks passed
@jthemphill jthemphill deleted the raytracing-gitignore branch September 28, 2023 05:46
@jthemphill
Copy link
Contributor Author

Sure! But since this now involves changing build.sh or the future build.py, I'm going to rebase this atop #3603 to avoid skew.

Oh, um... heads up that I had rebased this on #3603. This is because I thought #3603 would be merged first, and because the .gitignore depended on us storing everything in pkg, which happened in a change I made to build.py.

The end result of all this is that you have already merged most of #3603, probably without realizing it😬

@daxpedda
Copy link
Collaborator

... I sincerely apologize, I've somehow missed that, this is most definitely not your fault.
I will revert this so we can properly continue in #3603.

daxpedda added a commit that referenced this pull request Sep 28, 2023
@daxpedda
Copy link
Collaborator

@jthemphill if you don't mind re-opening this again after we finish in #3603, it would be appreciated.
Again, apologies!

daxpedda pushed a commit to jthemphill/wasm-bindgen that referenced this pull request Sep 28, 2023
@daxpedda
Copy link
Collaborator

I included this change in #3603, so you don't have to re-open this.

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.

3 participants