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

THRIFT-4764: Use new syntax for clippy skipping #1726

Closed
wants to merge 1 commit into from

Conversation

MikailBag
Copy link
Contributor

Client: rs

Pull Request

The Contributing Guide has more details and tips for
committing properly.

Review the following checklist to ensure a smooth pull request experience.

  • Did you squash your changes to a single commit?

  • Do you need an Apache Jira ticket?

    Expand for guidance...

    • Yes if your change requires a release note.
    • Yes if your change is a breaking change.
    • No if you change is trivial, such as fixing a typo.
  • Is this change worthy of a release note?
    Examples of Release Note-worthy examples...
    • Breaking Changes
    • New, Deprecated, or Removed Languages
    • Security Fixes
    • Significant Refactoring
    • Changing how the product is built
  • Breaking changes have additional requirements:
    Expand for instructions...
    • Add or reference an existing Apache Jira THRIFT ticket.
    • Add a Breaking-Change label to the Jira ticket.
    • Add a note to the lib/<language>/README.md file.
    • Add a line to the CHANGES.md file.
  • Does this change require a build?
    Expand for guidance...
    • Yes for any code change
    • Yes for any build script change
    • Yes for any docker build environment change
    • Yes for any change affecting the cross test suite
    • No for documentation-only changes
    • No for trivial changes, for example fixing a typo.

    If your change does not require a build, you can add [ci skip] to the end of your commit message.
    This will avoid costly and unnecessary builds in both the pull request and once it is merged.

For more information about committing, see the Contributing Guide.

Copy link
Contributor

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

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

From one build job:

�[0m�[1m�[38;5;9merror[E0658]�[0m�[0m�[1m: scoped lint `clippy::type_complexity` is experimental (see issue #44690)�[0m
�[0m�[0m�[1m�[36m    Building�[0m [====================================================>    ] 40/43
                                                                                
�[0m �[0m�[0m�[1m�[38;5;12m--> �[0m�[0msrc/thrift_test.rs:6:38�[0m
�[0m�[0m�[1m�[36m    Building�[0m [====================================================>    ] 40/43
                                                                                
�[0m  �[0m�[0m�[1m�[38;5;12m|�[0m
�[0m�[0m�[1m�[36m    Building�[0m [====================================================>    ] 40/43
                                                                                
�[0m�[1m�[38;5;12m6�[0m�[0m �[0m�[0m�[1m�[38;5;12m| �[0m�[0m#![allow(clippy::too_many_arguments, clippy::type_complexity)]�[0m
�[0m�[0m�[1m�[36m    Building�[0m [====================================================>    ] 40/43
                                                                                
�[0m  �[0m�[0m�[1m�[38;5;12m| �[0m�[0m                                     �[0m�[0m�[1m�[38;5;9m^^^^^^^^^^^^^^^^^^^^^^^�[0m

@mrwiggles
Copy link
Contributor

Sorry @jeking3 - been super busy after work for the past month :/

@mrwiggles
Copy link
Contributor

Looking now

@mrwiggles
Copy link
Contributor

@MikailBag All of the rust builds are failing, unfortunately. I suspect this is because the Rust version has to be bumped as well for the new clippy attributes to be understood.

@MikailBag
Copy link
Contributor Author

Oh, it seems that new syntax was only introduced in rustc v1.31, which was published 2018-12-06.

@jeking3
Copy link
Contributor

jeking3 commented Feb 7, 2019

If you can make it backwards compatible, great, otherwise we can't accept something that would push the minimum required version forward that far.

@MikailBag
Copy link
Contributor Author

Ok. Than I think I should close this PR, because I can't make changes backwards-compatible, and suppressing the lint is easy.

@MikailBag MikailBag closed this Feb 7, 2019
@allengeorge allengeorge reopened this Mar 28, 2020
@allengeorge
Copy link
Contributor

Since the Rust-version has been updated to 1.36, I believe we should be able to reapply this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants