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.

Sign up for GitHub

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

Jump to bottom

Run CI with latest (Rust 1.80), add ticket references to commented out tests #11661

Merged
merged 2 commits into from
Jul 26, 2024
Merged

Run CI with latest (Rust 1.80), add ticket references to commented out tests #11661

merged 2 commits into from
Jul 26, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jul 25, 2024 •
edited
Loading

Which issue does this PR close?

Closes #11657

Rationale for this change

  1. @findepi made a PR to pin CI to use Rust 1.79 while we work out the clippy failures: Temporarily pin toolchain version to avoid clippy errors #11655
  2. He then over achieved and brushed up @nrc's PR (that we sat on too long -- sorry 😿 ) and fixed the actual issues Fix clippy errors for Rust 1.80 #11654

Thus we should remove the temporary pin workaround

What changes are included in this PR?

  1. Revert the pin to use Rust 1.79
  2. Add ticket references for commented out tests

Are these changes tested?

By CI

Are there any user-facing changes?

No, this is all entirely development process

@alamb alamb added the development-process Related to development process of DataFusion label Jul 25, 2024
@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jul 25, 2024
@alamb alamb marked this pull request as ready for review July 25, 2024 20:13
This was referenced Jul 25, 2024
Closed
Merged
findepi
findepi reviewed Jul 25, 2024
View reviewed changes
datafusion/physical-plan/src/joins/hash_join.rs
@@ -1932,6 +1932,7 @@ mod tests {
}

// FIXME(#TODO) test fails with feature `force_hash_collisions`
// https://github.com/apache/datafusion/issues/11658
Copy link
Member

Choose a reason for hiding this comment

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

( #11658 )

findepi
findepi reviewed Jul 25, 2024
View reviewed changes
rust-toolchain.toml
# specific language governing permissions and limitations
# under the License.

[toolchain]
Copy link
Member

Choose a reason for hiding this comment

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

i meant to do this when rebasing #11654 after #11655 was merged. Was too slow :)

Thanks for cleaning this up!

@findepi
Copy link
Member

findepi commented Jul 26, 2024

@jayzhan211 can you please approve & merge?

comphead
comphead approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm! thanks @alamb

@comphead comphead merged commit 01dc3f9 into apache:main Jul 26, 2024
24 of 25 checks passed
@alamb alamb deleted the alamb/unpin_rust branch July 26, 2024 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findepi findepi findepi approved these changes

@jayzhan211 jayzhan211 jayzhan211 approved these changes

@comphead comphead comphead approved these changes

Assignees
No one assigned
Labels
development-process Related to development process of DataFusion sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Get Clippy clean for Rust 1.80 and run it on CI
4 participants
@alamb @findepi @comphead @jayzhan211