Skip to content

Conversation

@mqy
Copy link
Contributor

@mqy mqy commented Jan 8, 2021

The PR fixed cargo doc warnings (including the generated flatbuffer files), and corrected several typos.
To catch all possible cargo doc warnings, it looks like we have to run cargo doc for both nightly and stable.

I had filed an issue https://issues.apache.org/jira/browse/ARROW-11179 to address the cargo doc problem about generated flatbuffer files, hope this can be resolved soon. Currently, the patch is created as format-0ed34c83.patch for regen.sh.

@github-actions
Copy link

github-actions bot commented Jan 8, 2021

@mqy mqy changed the title ARROW-11168: [Rust] Fix cargo doc warnings ARROW-11168: [Rust] [Doc] Fix cargo doc warnings Jan 8, 2021
Copy link
Member

@jorgecarleitao jorgecarleitao 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 a lot for taking this!

@mqy mqy marked this pull request as draft January 8, 2021 07:32
@mqy
Copy link
Contributor Author

mqy commented Jan 8, 2021

Turn to draft because there are cargo test errors. Back soon.

@mqy mqy force-pushed the ARROW-11168_fix_cargo_warnings branch from 2a6c4ec to 2e7d35c Compare January 8, 2021 10:56
@mqy mqy marked this pull request as ready for review January 8, 2021 11:06
@mqy mqy requested a review from jorgecarleitao January 8, 2021 11:06
@mqy mqy marked this pull request as draft January 8, 2021 12:03
@mqy mqy force-pushed the ARROW-11168_fix_cargo_warnings branch from e630a99 to 78eaa87 Compare January 8, 2021 13:01
@mqy
Copy link
Contributor Author

mqy commented Jan 8, 2021

The build failed, no idea why does it build parquet-format 2.7.0.
I can't reproduce this problem with cargo build --all-targets on my local host with code of this PR.

The merged commit c4ee536 also failed with same error.

The problem is filed at https://issues.apache.org/jira/browse/ARROW-11184

@nevi-me I saw you just released parquet-format 2.7.0 hours ago, any idea?

@mqy mqy marked this pull request as ready for review January 8, 2021 13:50
@nevi-me
Copy link
Contributor

nevi-me commented Jan 8, 2021

We released parquet-format-rs:2.7.0 today. If it's breaking this build, then it's likely going to break others. I'll have a look, and pin versions if needed.

@alamb
Copy link
Contributor

alamb commented Jan 8, 2021

@alamb
Copy link
Contributor

alamb commented Jan 8, 2021

I'll look into this and try and get the build passing again

@nevi-me
Copy link
Contributor

nevi-me commented Jan 8, 2021

@alamb I've opened #9138

@alamb
Copy link
Contributor

alamb commented Jan 8, 2021

This PR probably needs to be rebased to pick up the fix for #9138

@mqy mqy force-pushed the ARROW-11168_fix_cargo_warnings branch from 9208930 to 8cf807f Compare January 8, 2021 19:48
@mqy
Copy link
Contributor Author

mqy commented Jan 8, 2021

LGTM. Thanks a lot for taking this!

@jorgecarleitao thanks for the review. It seems the CI is getting close to success, please review again.

@alamb alamb closed this in 08cccd6 Jan 9, 2021
@alamb
Copy link
Contributor

alamb commented Jan 9, 2021

FWIW I tested this branch out locally and it worked really nicely -- no more warnings during build. Thanks @mqy !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants