search

apache / arrow-rs

Official Rust implementation of Apache Arrow
https://arrow.apache.org/
Apache License 2.0
2.51k stars 746 forks source link

Looking for help maintaining (reviewing PRs) in arrow/parquet/object_store #6418

Open alamb opened 1 week ago

alamb commented 1 week ago

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

PRs in this repository are now taking days/weeks to review, which results in

  1. Slower velocity (how fast we can change this library)
  2. A bad community experience (if you get no feedback for a while, it is likely you will go elsewhere to help)

This repo isn't something "we" are providing or are gate keeping. This is project is a community effort in all aspects.

Describe the solution you'd like I would like more capacity to review PRs in this repo so that we can merge them faster and accelerate progress in this repository.

Reviewing PRs means;

  1. Ensure all PRs are looked at in a timely manner (ideally within 24 hours) (see list here)
  2. Ensure the PR follow the contribution guidelines.
  3. Ensure the PR is clearly documented, and the code follows existing patterns
  4. Ensure the new functionality is covered with tests
  5. When it looks good, @ mention one of the committers and we'll give it a review and merge

Describe alternatives you've considered I would like everyone who contributes code to this repository to feel both welcome and empowered to review PRs. It takes time and effort, no doubt, but you will very likely learn something and sustained help reviewing PRs is a great way to be considered for committer yourself.

If you are waiting on a review, please consider helping review other PRs -- it will reduce the time before your PR is reviewed.

People may have the idea that PRs can only be reviewed by "committers" in arrow. However, this is not the case -- committers need to do a final approve + merge, but anyone can (and should!) be reviewing other PRs.

As an example, @iffyio's work in sqlparser-rs is textbook example of how to review PRs: https://github.com/sqlparser-rs/sqlparser-rs/pulls?q=is%3Apr+commenter%3Aiffyio

Additional context Previously @tustvold spent a significant amount of his effort on reviewing PRs in this crate and was paid to do so by InfluxData. He has now moved on to other endeavors, and while he is still helping and InfluxData is working on backfilling his review capacity this project should not be seen as something we can afford to maintain by ourselves

alamb commented 1 week ago

BTW total kudos to @Xuanwo for their help so far -- it has been most appreciated

ByteBaker commented 1 week ago

@alamb I'd be happy to chip in. Especially around the extremely older ones. Because I feel some of them have stalled to the point where they're effectively obsolete but still keeping an entry in open issues' list.

Xuanwo commented 1 week ago

BTW total kudos to @Xuanwo for their help so far -- it has been most appreciated

Please feel free to ping me if there are object_store or storage-related PRs (such as the parquet async reader) that need my review, as I'm confident in handling them.

For other parts of this project, I'm still learning, so I can only handle the simpler ones.

alamb commented 1 week ago

@alamb I'd be happy to chip in. Especially around the extremely older ones. Because I feel some of them have stalled to the point where they're effectively obsolete but still keeping an entry in open issues' list.

That would be super helpful @ByteBaker -- indeed some of the older ones are stalled indeed. Any help cleaning would be helpful (even if it is just a ping to me or another committer saying they should probably be closed)

alamb commented 1 week ago

@dsgibbons asked on discord:

Is there a list of relevant committers for arrow-rs specifically? (as opposed to C++, Go etc.)

TLDR there is no such list I know of or official distinction between arrow commiters; However, clearly there are those of us who focus more on the rust implementation,

I think the easiest thing to do is look at who has merged recent PRs in this repo. Examples: myself @alamb , @tustvold , @viirya , @Dandandan )

bkietz commented 1 week ago

It might be worthwhile to include https://github.com/google/flatbuffers/issues/8150 on this list ( PRs https://github.com/google/flatbuffers/pull/8398 https://github.com/apache/arrow-rs/pull/6426 )

alamb commented 1 week ago

It might be worthwhile to include google/flatbuffers#8150 on this list ( PRs google/flatbuffers#8398 #6426 )

@bkietz what list are you referring to?

bkietz commented 1 week ago

I meant the list of PRs linked to this issue above

alamb commented 1 hour ago

BTW there has been a marked uptick in review activity, thank you to @etseidl @wiedld @Xuanwo @westonpace @ByteBaker and others who I probably forgot for all their help 🙏