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

chore: fix wasm_bindgen_test macros #2675

Merged
merged 4 commits into from Aug 17, 2023
Merged

chore: fix wasm_bindgen_test macros #2675

merged 4 commits into from Aug 17, 2023

Conversation

hlbarber
Copy link
Contributor

@hlbarber hlbarber commented Aug 6, 2023 •
edited

Motivation

Tests involving wasm_bindgen_test currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

Solution

@hlbarber hlbarber changed the title chore: fix wasm_bindgen_test_macros chore: fix wasm_bindgen_test macros Aug 6, 2023
@hlbarber hlbarber marked this pull request as ready for review August 6, 2023 22:18
@hlbarber hlbarber requested review from hawkw, davidbarsky and a team as code owners August 6, 2023 22:18
@hawkw
Copy link
Member

hawkw commented Aug 6, 2023

hmm, looks like CI is still failing?

@hlbarber
Copy link
Contributor Author

hlbarber commented Aug 7, 2023 •
edited

hmm, looks like CI is still failing?

Yeah, we'll have to wait for rustwasm/wasm-bindgen#3549 to be released.

The failures are of the form

error[E0425]: cannot find value `None` in this scope
   --> tracing/tests/macros.rs:946:36
    |
946 | #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not found in this scope

@hlbarber
Copy link
Contributor Author

hlbarber commented Aug 9, 2023

If you want a quick fix I can just pull Option::Some and None into scope (for only target_arch = wasm32 perhaps).

@davidbarsky
Copy link
Member

If you want a quick fix I can just pull Option::Some and None into scope (for only target_arch = wasm32 perhaps).

Not necessary yet. I'll ask around first.

@hawkw
Copy link
Member

hawkw commented Aug 14, 2023

I guess in the meantime, I'm willing to merge PRs while waiting for upstream changes...

@davidbarsky
Copy link
Member

given that I got no response, I suppose we should just merge this/potentially with Option being imported.

@hawkw
Copy link
Member

hawkw commented Aug 14, 2023

given that I got no response, I suppose we should just merge this/potentially with Option being imported.

cool, okay --- @hlbarber, can i get you to add the import, then?

@hawkw hawkw mentioned this pull request Aug 16, 2023
Merged
@hlbarber
Copy link
Contributor Author

@hawkw done

hawkw
hawkw approved these changes Aug 17, 2023
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks great, thanks for working on this @hlbarber!

@hawkw hawkw merged commit 6c530d5 into tokio-rs:master Aug 17, 2023
56 checks passed
@hlbarber hlbarber deleted the fix-wasm-macros branch September 11, 2023 23:58
davidbarsky pushed a commit that referenced this pull request Sep 26, 2023
@hlbarber @davidbarsky
tracing: fix wasm_bindgen_test macros (#2675)
ab27b7a 
## Motivation

Tests involving `wasm_bindgen_test` currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

## Solution

- [x] Use `extern crate wasm_bindgen_test` to side-step `no_implicit_prelude`.
- [x] rustwasm/wasm-bindgen#3549
- [ ] Consume the release `wasm_bindgen_test` containing said change.
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@hlbarber @davidbarsky
tracing: fix wasm_bindgen_test macros (#2675)
7b19f32 
## Motivation

Tests involving `wasm_bindgen_test` currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

## Solution

- [x] Use `extern crate wasm_bindgen_test` to side-step `no_implicit_prelude`.
- [x] rustwasm/wasm-bindgen#3549
- [ ] Consume the release `wasm_bindgen_test` containing said change.
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@hlbarber @davidbarsky
tracing: fix wasm_bindgen_test macros (#2675)
161c9cc 
## Motivation

Tests involving `wasm_bindgen_test` currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

## Solution

- [x] Use `extern crate wasm_bindgen_test` to side-step `no_implicit_prelude`.
- [x] rustwasm/wasm-bindgen#3549
- [ ] Consume the release `wasm_bindgen_test` containing said change.
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
@hlbarber @davidbarsky
tracing: fix wasm_bindgen_test macros (#2675)
5da7799 
## Motivation

Tests involving `wasm_bindgen_test` currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

## Solution

- [x] Use `extern crate wasm_bindgen_test` to side-step `no_implicit_prelude`.
- [x] rustwasm/wasm-bindgen#3549
- [ ] Consume the release `wasm_bindgen_test` containing said change.
hawkw pushed a commit that referenced this pull request Oct 1, 2023
@hlbarber @hawkw
tracing: fix wasm_bindgen_test macros (#2675)
3cde4fa 
## Motivation

Tests involving `wasm_bindgen_test` currently fail:

https://github.com/tokio-rs/tracing/actions/runs/5756318807/job/15605512576

## Solution

- [x] Use `extern crate wasm_bindgen_test` to side-step `no_implicit_prelude`.
- [x] rustwasm/wasm-bindgen#3549
- [ ] Consume the release `wasm_bindgen_test` containing said change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

@hawkw hawkw hawkw 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.

None yet
3 participants
@hlbarber @hawkw @davidbarsky