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

Disable wgpu-core documentation as a workaround for #4905. #5987

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Jul 18, 2024

Connections
Proposed workaround for #4905.

Description
This enables cargo doc to succeed in a reasonable amount of time, as long as the reader isn't looking for documentation for wgpu-core itself.

Testing
Manually ran cargo doc and observed it succeeded, and checked the resulting wgpu-core and wgpu documentation.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@kpreid kpreid requested a review from a team as a code owner July 18, 2024 23:06
@kpreid kpreid force-pushed the explode-the-core branch 2 times, most recently from f4858e1 to 61e28ce Compare July 18, 2024 23:09
@cwfitzgerald
Copy link
Member

Ah, nice workaround, we can also re-enable building docs in CI (currently also commented out)

@cwfitzgerald
Copy link
Member

Would it be possible to add re-enabling CI to this PR?

@kpreid
Copy link
Contributor Author

kpreid commented Jul 19, 2024

Would it be possible to add re-enabling CI to this PR?

Besides not building docs, there's a bunch of stale workaround stuff (just search for “4905” in comments), so I'd rather leave that to a separate PR in case of further messes.

@kpreid
Copy link
Contributor Author

kpreid commented Jul 19, 2024

Oh, wait, I see — if we just enable the docs build now, then this PR gets that aspect tested. Okay.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Amazing, thanks!

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) July 19, 2024 04:46
auto-merge was automatically disabled July 19, 2024 05:12

Head branch was pushed to by a user without write access

@kpreid
Copy link
Contributor Author

kpreid commented Jul 19, 2024

Fixed changelog merge conflict.

Adjusting the CI script made me think: perhaps the “document wgpu-core anyway” should actually not be a feature but a bare cfg, so that --all-features doesn't enable it. But that means giving users who want the docs the pain of RUSTFLAGS.

@cwfitzgerald
Copy link
Member

Hmmm, I think I like the cfg better as wanting this is going to be rare as it's so un-useful

This enables `cargo doc` to succeed in a reasonable amount of time,
as long as the reader isn't looking for documentation for `wgpu-core`
itself.
This reverts most of the changes in a2fcd72,
but the "document private features" step is still disabled since it
operates only on wgpu-core which is exactly the problem.
@kpreid
Copy link
Contributor Author

kpreid commented Jul 19, 2024

Hmmm, I think I like the cfg better as wanting this is going to be rare as it's so un-useful

Done.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Noice

@cwfitzgerald cwfitzgerald merged commit 56d418f into gfx-rs:trunk Jul 19, 2024
25 checks passed
@kpreid kpreid deleted the explode-the-core branch July 26, 2024 16:59
@teoxoy teoxoy added the PR: needs back-porting PR with a fix that needs to land on crates label Jul 26, 2024
cwfitzgerald pushed a commit to cwfitzgerald/wgpu that referenced this pull request Jul 31, 2024
…rs#5987)

This enables `cargo doc` to succeed in a reasonable amount of time,
as long as the reader isn't looking for documentation for `wgpu-core`
itself.

(cherry picked from commit b5934e8)

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: needs back-porting PR with a fix that needs to land on crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants