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

codewide: appease clippy again #1046

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Conversation

wprzytula
Copy link
Collaborator

This time clippy got angry about bad indentation in docstrings' lists.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [ ] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have provided docstrings for the public items that I want to introduce.
  • [ ] I have adjusted the documentation in ./docs/source/.
  • [ ] I added appropriate Fixes: annotations to PR description.

This time it got angry about bad indentation in docstrings' lists.
@wprzytula wprzytula requested review from muzarski and Lorak-mmk July 29, 2024 15:55
@Lorak-mmk
Copy link
Collaborator

I approved the current changes, but they seem to not be enough to make clippy happy.

@wprzytula
Copy link
Collaborator Author

I approved the current changes, but they seem to not be enough to make clippy happy.

The remaining errors come not from clippy, but from rustc itself. A new version of rustc denied support for arbitrary cfg parameters. Those must be declared, as the error message says.

I would merge this as-is, solving the cfg issue in another PR, because it has nothing to do with clippy.

@wprzytula wprzytula self-assigned this Jul 30, 2024
@Lorak-mmk
Copy link
Collaborator

I approved the current changes, but they seem to not be enough to make clippy happy.

The remaining errors come not from clippy, but from rustc itself. A new version of rustc denied support for arbitrary cfg parameters. Those must be declared, as the error message says.

I would merge this as-is, solving the cfg issue in another PR, because it has nothing to do with clippy.

Is there a reason for doing this in another PR? I'm reluctant to merge without passing CI. Can't this PR fix both issues?

@wprzytula
Copy link
Collaborator Author

Is there a reason for doing this in another PR?

Just naming, and keeping PRs doing one thing, not multiple.

I'm reluctant to merge without passing CI.

I see no point in being that religious about passing CI. If the only changes the PR introduces are related to indentation in docstrings, nothing can get worse because of the PR.

Can't this PR fix both issues?

Well, it can. Any PR can fix any number of issues.

@Lorak-mmk Lorak-mmk merged commit 34352da into scylladb:main Jul 30, 2024
9 of 11 checks passed
@wprzytula wprzytula deleted the appease-clippy branch August 20, 2024 08:07
wprzytula pushed a commit to wprzytula/scylla-rust-driver that referenced this pull request Aug 22, 2024
codewide: appease clippy again
(cherry picked from commit 34352da)
@wprzytula wprzytula mentioned this pull request Aug 22, 2024
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