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

tiny_skia_render: Fix unused qualifications. #62

Merged
merged 1 commit into from
May 23, 2024

Conversation

waywardmonkeys
Copy link
Contributor

No description provided.

@waywardmonkeys waywardmonkeys requested a review from nicoburns May 23, 2024 13:25
@nicoburns nicoburns added this pull request to the merge queue May 23, 2024
Merged via the queue into linebender:main with commit f7f339b May 23, 2024
19 checks passed
@xStrom
Copy link
Member

xStrom commented May 23, 2024

This is a bit of a head scratcher.

The merge queue CI did

git fetch --no-tags --prune --no-recurse-submodules --depth=1 origin +94c5d78214b0a2c50d0c3e11d700a0e38080e7b9:refs/remotes/origin/gh-readonly-queue/main/pr-55-c982ffc22e861f66419b43b7323fea3754085c6d

which seems to correctly already have this lint. However it didn't trigger during the clippy run. 😕

@waywardmonkeys
Copy link
Contributor Author

This happened in my multi-crate PR because that leaves examples at the top level so they're now more correctly compiled independently, I think. But now I'm having other CI troubles in that PR.

@waywardmonkeys waywardmonkeys deleted the unused-qualifications-v2 branch May 23, 2024 13:49
@xStrom
Copy link
Member

xStrom commented May 23, 2024

Well the thing is that the following fails locally for me:

git checkout 94c5d78214b0a2c50d0c3e11d700a0e38080e7b9
cargo hack clippy -p tiny_skia_render --locked --optional-deps --each-feature

Which is what the merge queue CI allegedly did.

@waywardmonkeys
Copy link
Contributor Author

Well, on a related topic maybe, I was getting a warning about the unused StyleData when building locally since before the release, yet it apparently never failed in CI.

@nicoburns
Copy link
Contributor

Yeah, I had a bunch of issues with CI checks not matching local checks with my PRs. I'm not entirely convinced by "cargo hack", although manually writing out feature combinations isn't great either.

@xStrom xStrom mentioned this pull request May 23, 2024
xStrom added a commit to xStrom/parley that referenced this pull request May 23, 2024
This reverts commit 49e055a.

Revert "Add automatic CI support for packages without the `std` feature. (linebender#63)"

This reverts commit eb2c68a.

Revert "tiny_skia_render: Fix unused qualifications. (linebender#62)"

This reverts commit f7f339b.
@xStrom
Copy link
Member

xStrom commented May 23, 2024

Manually maintaining the package + feature matrix is untenable. Cargo hack has served us well. I have found some issues, but its maintainer is very responsive and accepted my patches. So if you find any issues with cargo hack, definitely let me know.

As for this issue right here, it has nothing to do with cargo hack.

Turns out that this unused_qualifications lint requires 1.78 and the Parley CI is still configured to run 1.77. So no weirdness, just a tiny bit stale config.

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.

3 participants