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

Remove unnecessary status check #798

Merged
merged 1 commit into from
May 4, 2020
Merged

Remove unnecessary status check #798

merged 1 commit into from
May 4, 2020

Conversation

lntotk
Copy link
Contributor

@lntotk lntotk commented Apr 24, 2020

In Table::Open function, we already judge status just after Decode footer, so the status judge before read index block is duplicate and unnessary, we can remove it.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@lntotk
Copy link
Contributor Author

lntotk commented Apr 24, 2020

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@lntotk
Copy link
Contributor Author

lntotk commented Apr 24, 2020

is the mac clang configuration have some error?

@pwnall pwnall self-requested a review May 4, 2020 09:30
Copy link
Member

@pwnall pwnall left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for this contribution!

I've kicked off importing this PR in google3 (our internal repository). The PR will eventually be merged by our bots.

@pwnall pwnall changed the title remove unnessary status judge Remove unnecessary status check May 4, 2020
@pwnall
Copy link
Member

pwnall commented May 4, 2020

And to answer the question above.. the CI was fixed recently. Rebasing on top of master would've given you a green check here.

At this point, however, it's better to leave this PR as-is. Messing with it may confuse the bot.

@pwnall pwnall merged commit 5bd5f0f into google:master May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants