Skip to content

Remove (and deny) useless test vec!s #3883

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

Merged
merged 2 commits into from
Jun 26, 2025

Conversation

TheBlueMatt
Copy link
Collaborator

Had this one lying around after #3851 so figured its worth doing before it gets stale.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jun 23, 2025

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz June 23, 2025 12:34
tnull
tnull previously approved these changes Jun 23, 2025
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM. Happy to land as soon as CI let's us, as it's trivial.

@tnull tnull removed the request for review from jkczyz June 23, 2025 13:35
@tnull
Copy link
Contributor

tnull commented Jun 23, 2025

@TheBlueMatt seems Github is of the opinion this needs a rebase already. Also CI is hanging in a weird way here.

In the next commit we'll set clippy to ban useless `vec!`s in
tests, so here we remove them.
There's no reason to allow these, and now that they've been
removed, we should simply deny them in tests like we do in normal
code.
@TheBlueMatt TheBlueMatt force-pushed the 2025-06-no-test-useless-vecs branch from 0af6620 to 6091c4a Compare June 25, 2025 18:03
@TheBlueMatt TheBlueMatt requested a review from tnull June 26, 2025 11:00
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM, landing.

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@tnull tnull merged commit 15724ec into lightningdevkit:main Jun 26, 2025
27 of 28 checks passed
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