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.

Sign up for GitHub

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

Jump to bottom

Rename WGPUDeviceLostReason_Undefined to Unknown #288

Merged
merged 2 commits into from
May 18, 2024
Merged

Rename WGPUDeviceLostReason_Undefined to Unknown #288

merged 2 commits into from
May 18, 2024

Conversation

hocheung-chromium
Copy link
Contributor

@hocheung-chromium hocheung-chromium commented May 9, 2024 •
edited by kainino0x
Loading

Please check
https://issues.chromium.org/issues/42241081 for
more information and progress.

@kainino0x @austinEng

Fixes #223

hocheung-chromium added a commit to hocheung-chromium/emscripten that referenced this pull request May 9, 2024
@hocheung-chromium
Rename WGPUDeviceLostReason_Undefined to Unknown
0fe8a8d 
Please check
https://issues.chromium.org/issues/42241081 for
more information and progress.

webgpu-headers PR:
webgpu-native/webgpu-headers#288.
@hocheung-chromium hocheung-chromium mentioned this pull request May 9, 2024
Merged
@austinEng
Copy link
Collaborator

I believe WGPUDeviceLostReason_Unknown should start with value 1, not 0

hocheung-chromium added a commit to hocheung-chromium/emscripten that referenced this pull request May 11, 2024
@hocheung-chromium
Rename WGPUDeviceLostReason_Undefined to Unknown
3f6926d 
Please check
https://issues.chromium.org/issues/42241081 for
more information and progress.

webgpu-headers PR:
webgpu-native/webgpu-headers#288.
@hocheung-chromium
Copy link
Contributor Author

hocheung-chromium commented May 11, 2024 •
edited
Loading

I believe WGPUDeviceLostReason_Unknown should start with value 1, not 0

I have redefined the values ​​for Unknown and Destoryed in the yml file

@austinEng

@hocheung-chromium
Copy link
Contributor Author

Hello Kai Ninomiya, could you please take a moment to review this PR? Thanks.

@kainino0x

hocheung-chromium added a commit to hocheung-chromium/emscripten that referenced this pull request May 15, 2024
@hocheung-chromium
Rename WGPUDeviceLostReason_Undefined to Unknown
83f9f82 
Please check
https://issues.chromium.org/issues/42241081 for
more information and progress.

webgpu-headers PR:
webgpu-native/webgpu-headers#288.
hocheung-chromium added a commit to hocheung-chromium/emscripten that referenced this pull request May 16, 2024
@hocheung-chromium
Rename WGPUDeviceLostReason_Undefined to Unknown
428fea3 
Please check
https://issues.chromium.org/issues/42241081 for
more information and progress.

webgpu-headers PR:
webgpu-native/webgpu-headers#288.
@hocheung-chromium
Copy link
Contributor Author

I also followed your instructions and kept Undefined here. Please review, thank you.

@kainino0x

kainino0x
kainino0x reviewed May 17, 2024
View reviewed changes
Copy link
Collaborator

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

I'll link #223 from the summary.

webgpu.yml Outdated Show resolved Hide resolved
webgpu.h Outdated Show resolved Hide resolved
@kainino0x
Copy link
Collaborator

I also followed your instructions and kept Undefined here. Please review, thank you.

As mentioned we don't actually need it for this repository. Sorry the state of things is confusing right now. Also apologies for the review delay!

I removed Undefined, so LGTM!

@gz83
Copy link

gz83 commented May 18, 2024

I also followed your instructions and kept Undefined here. Please review, thank you.

As mentioned we don't actually need it for this repository. Sorry the state of things is confusing right now. Also apologies for the review delay!

I removed Undefined, so LGTM!

I see what you mean, thank you very much for reviewing my PR.

@kainino0x

@rajveermalviya rajveermalviya merged commit 8049c32 into webgpu-native:main May 18, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kainino0x kainino0x kainino0x approved these changes

@austinEng austinEng austinEng approved these changes

@rajveermalviya rajveermalviya rajveermalviya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rename WGPUDeviceLostReason "Undefined" to "Unknown"
5 participants
@hocheung-chromium @austinEng @kainino0x @gz83 @rajveermalviya