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

add panic message #2259

Merged
merged 3 commits into from Sep 6, 2017

Conversation

Projects
None yet
6 participants
@huachaohuang
Member

huachaohuang commented Sep 6, 2017

No description provided.

@lishuai87

LGTM

@@ -252,6 +252,7 @@ impl IndexHandles {
}
fn get_approximate_distance_in_range(&self, start: &[u8], end: &[u8]) -> u64 {
assert!(end >= start);

This comment has been minimized.

@ngaut

ngaut Sep 6, 2017

Member

Does equal condition exist?

@ngaut

ngaut Sep 6, 2017

Member

Does equal condition exist?

This comment has been minimized.

@huachaohuang

huachaohuang Sep 6, 2017

Member

Doesn't exist for now. But from the perspective of this function, it can support such a condition.

@huachaohuang

huachaohuang Sep 6, 2017

Member

Doesn't exist for now. But from the perspective of this function, it can support such a condition.

@@ -266,7 +267,15 @@ impl IndexHandles {
v.offset
}
};
assert!(end_offset >= start_offset);
if end_offset < start_offset {

This comment has been minimized.

@andelf

andelf Sep 6, 2017

Contributor

If we have guidelines of when to panic! and when to assert!?

assert!(end_offset >= start_offset, "start {:?} end {:?} start_offset {} end_offset {}",
                start,
                end,
                start_offset,
                end_offset
            );
@andelf

andelf Sep 6, 2017

Contributor

If we have guidelines of when to panic! and when to assert!?

assert!(end_offset >= start_offset, "start {:?} end {:?} start_offset {} end_offset {}",
                start,
                end,
                start_offset,
                end_offset
            );

This comment has been minimized.

@BusyJay

BusyJay Sep 6, 2017

Contributor

We prefer explicit panic in general sources, and assert in tests.

@BusyJay

BusyJay Sep 6, 2017

Contributor

We prefer explicit panic in general sources, and assert in tests.

This comment has been minimized.

@huachaohuang

huachaohuang Sep 6, 2017

Member

Oh, I don't know assert! can be used like that, I prefer to use assert! here.

@huachaohuang

huachaohuang Sep 6, 2017

Member

Oh, I don't know assert! can be used like that, I prefer to use assert! here.

@huachaohuang

This comment has been minimized.

Show comment
Hide comment
@huachaohuang

huachaohuang Sep 6, 2017

Member

/rebuild

Member

huachaohuang commented Sep 6, 2017

/rebuild

@huachaohuang

This comment has been minimized.

Show comment
Hide comment
@huachaohuang

huachaohuang Sep 6, 2017

Member

/run-test

Member

huachaohuang commented Sep 6, 2017

/run-test

@huachaohuang huachaohuang merged commit ce30d60 into master Sep 6, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@huachaohuang huachaohuang deleted the huachao/assert branch Sep 6, 2017

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