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

minor naga span updates #7029

Merged
merged 5 commits into from
Jan 30, 2025
Merged

Conversation

brody4hire
Copy link
Contributor

Connections

trigger is reducing dependency on std::error::Error for other PR: #6938

(help with no-std support as requested in issue: #6826)

Description

  • move internal naga span::AddSpan code to keep internal AddSpan trait & impl together
  • remove where dependency on std::error::Error from AddSpan impl
  • add blank lines & returned output type comments to AddSpan & MapErrWithSpan traits
  • update MapErrWithSpan to be pub(crate) trait

NOTE: I am submitting this with multiple commits to hopefully keep these proposed updates as clear as possible. I would be happy to squash if needed. I would also be happy to revert the MapErrWithSpan trait visibility update if needed.

Testing

see checklist below

Checklist

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

@@ -351,36 +377,12 @@ impl<T> SpanProvider<T> for UniqueArena<T> {
}
}

impl<E> AddSpan for E
where
E: Error,
Copy link
Member

Choose a reason for hiding this comment

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

Wow this was just flat out not needed?

@cwfitzgerald
Copy link
Member

NOTE: I am submitting this with multiple commits to hopefully keep these proposed updates as clear as possible.

Sounds good, will rebase-and-merge these.

I would be happy to squash if needed.

We always have the squash-and-merge button, so feel free to be as messy with commits as you want, as long as you want us to review as a whole as opposed to commit-by-commit.

@cwfitzgerald cwfitzgerald merged commit 55c33b0 into gfx-rs:trunk Jan 30, 2025
31 checks passed
@brody4hire brody4hire deleted the minor-naga-span-updates branch January 30, 2025 02:24
@brody4hire
Copy link
Contributor Author

brody4hire commented Jan 30, 2025

@cwfitzgerald it looks to me like you did some kind of rebase merge, with 4 commits added to trunk now. Not a big deal but not as I had expected

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.

2 participants