Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Feb 16, 2021

This PR is based off of #9425 from @jorgecarleitao with some proposed resolutions to comments by @sunchao

@github-actions
Copy link

@alamb
Copy link
Contributor Author

alamb commented Feb 26, 2021

Plan is to rebase anything useful out of this PR into a new ticket -- we merged the original PR as is #9425

@alamb
Copy link
Contributor Author

alamb commented Mar 13, 2021

I cleaned these changes up into #9690 and #9691 so closing this one

@alamb alamb closed this Mar 13, 2021
alamb added a commit that referenced this pull request Mar 14, 2021
# Background:
Left over cleanups suggested by from @sunchao on  #9425

Broken out from #9508

# Rationale:
This function is redundant with `OffsetSize::is_large`

Closes #9690 from alamb/alamb/remove_prefix

Authored-by: Andrew Lamb <[email protected]>
Signed-off-by: Andrew Lamb <[email protected]>
alamb added a commit that referenced this pull request Mar 15, 2021
…ead of `panic!`

# Background:
Left over cleanups suggested by from @sunchao on  #9425

Broken out from #9508

# Rationale:

Don't use panic! directly. However,  since the caller of this function still calls `unwrap()`, I am not sure how much of an improvement this change really is. However it may set us up for a more `safe` future eventually

Closes #9691 from alamb/alamb/fallable_list_conversion

Authored-by: Andrew Lamb <[email protected]>
Signed-off-by: Andrew Lamb <[email protected]>
alamb added a commit to apache/arrow-rs that referenced this pull request Apr 20, 2021
# Background:
Left over cleanups suggested by from @sunchao on  apache/arrow#9425

Broken out from apache/arrow#9508

# Rationale:
This function is redundant with `OffsetSize::is_large`

Closes #9690 from alamb/alamb/remove_prefix

Authored-by: Andrew Lamb <[email protected]>
Signed-off-by: Andrew Lamb <[email protected]>
alamb added a commit to apache/arrow-rs that referenced this pull request Apr 20, 2021
…ead of `panic!`

# Background:
Left over cleanups suggested by from @sunchao on  apache/arrow#9425

Broken out from apache/arrow#9508

# Rationale:

Don't use panic! directly. However,  since the caller of this function still calls `unwrap()`, I am not sure how much of an improvement this change really is. However it may set us up for a more `safe` future eventually

Closes #9691 from alamb/alamb/fallable_list_conversion

Authored-by: Andrew Lamb <[email protected]>
Signed-off-by: Andrew Lamb <[email protected]>
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.

2 participants