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

Enable transparencyAsCoverage in Babylon Viewer #4531

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

bghgary
Copy link
Contributor

@bghgary bghgary commented Oct 24, 2023

@google-cla
Copy link

google-cla bot commented Oct 24, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@elalish
Copy link
Collaborator

elalish commented Oct 25, 2023

Thank you! Is it working? You may want to update the babylon.js version while you're at it; it's probably quite out of date.

@bghgary
Copy link
Contributor Author

bghgary commented Oct 25, 2023

No idea. I am not having luck getting it to run locally. I was hoping the CI can run it instead.

@bghgary
Copy link
Contributor Author

bghgary commented Oct 27, 2023

@elalish I am unable to get the cla/google to pass since I only use the users.noreply.github.com email in my commits which doesn't work apparently. Maybe you can take it further?

EDIT: Never mind. It works. I just need to rescan.

@bghgary bghgary marked this pull request as ready for review October 27, 2023 17:02
@bghgary
Copy link
Contributor Author

bghgary commented Oct 27, 2023

I don't know what full_test_run is and why it's waiting.

You may want to update the babylon.js version while you're at it; it's probably quite out of date.

Maybe as a separate PR to limit the changes.

@elalish
Copy link
Collaborator

elalish commented Oct 27, 2023

Oh, don't worry about full_test_run - you're not changing anything in that code anyway. I need to update our CI for contributors. And it looks like @bhouston updated Babylon in #4535, so we should be good.

Copy link
Collaborator

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Thanks!

@elalish
Copy link
Collaborator

elalish commented Oct 27, 2023

So, our CI does't actually run any renderers besides <model-viewer>. Let's let a few more of @bhouston's PRs land and that should make it easier for you to update goldens.

@bghgary
Copy link
Contributor Author

bghgary commented Oct 27, 2023

that should make it easier for you to update goldens.

Hmm, I don't see any information on this, so I have no idea what you are talking about.

@elalish
Copy link
Collaborator

elalish commented Oct 27, 2023

#4542

@bghgary
Copy link
Contributor Author

bghgary commented Oct 27, 2023

Ok, just tell me what I need to do. I can't even get the npm run test to work locally.

@elalish
Copy link
Collaborator

elalish commented Oct 27, 2023

Okay, if your dev environment isn't working then I'll assume you know how to write Babylon code like this blindly. I'll go ahead and merge it. If @bhouston has any trouble making babylon goldens, he can ping you here or there.

@elalish elalish merged commit 49fd9e4 into google:master Oct 27, 2023
2 checks passed
@bghgary
Copy link
Contributor Author

bghgary commented Oct 27, 2023

I would definitely prefer for it to work, but let me know if something goes wrong with this change.

JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
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.

None yet

2 participants