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.

Sign up for GitHub

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

Jump to bottom

feat: add --from-page flag to check cmd #737

Merged
merged 1 commit into from
Apr 23, 2024
Merged

feat: add --from-page flag to check cmd #737

merged 1 commit into from
Apr 23, 2024

Conversation

Elbehery
Copy link
Member

@Elbehery Elbehery commented Apr 21, 2024 •
edited by ahrtr
Loading

This PR enhances the cobra style check cmd.

It adds the ability to check the db integrity starting from specific pageId instead of the whole db file.

Link to #580

cc @ahrtr @tjungblu @fuweid @ivanvc

@Elbehery Elbehery mentioned this pull request Apr 21, 2024
Closed
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch from 421a862 to 93e1bbb Compare April 21, 2024 15:41
@Elbehery Elbehery changed the title feat: add check pgid sub-command feat: add check-page sub-command Apr 21, 2024
ivanvc
ivanvc reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

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

I left a couple of comments. Mainly regarding the implementation of this as a sub-command or not.

cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch 3 times, most recently from aa45469 to 98f38f4 Compare April 22, 2024 10:44
@ahrtr
Copy link
Member

ahrtr commented Apr 22, 2024

It seems that you amended your commit into the latest commit in main branch?

@Elbehery
Copy link
Member Author

It seems that you amended your commit into the latest commit in main branch?

so sorry, i was rebasing the main before committing

fixing ...

@Elbehery Elbehery changed the title feat: add check-page sub-command feat: add page-Id flag to check cmd Apr 22, 2024
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch from 98f38f4 to 9b0e668 Compare April 22, 2024 18:43
@Elbehery
Copy link
Member Author

@ahrtr ptal 👍🏽

ivanvc
ivanvc reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

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

Thanks, @Elbehery, for addressing the suggestion. I left another two comments :)

cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
cmd/bbolt/command_check_test.go Outdated Show resolved Hide resolved
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch from 9b0e668 to acd7d05 Compare April 22, 2024 19:29
ahrtr
ahrtr reviewed Apr 23, 2024
View reviewed changes
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch 2 times, most recently from 661a285 to a262b1e Compare April 23, 2024 14:48
ahrtr
ahrtr reviewed Apr 23, 2024
View reviewed changes
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch from a262b1e to 94152c1 Compare April 23, 2024 17:20
ahrtr
ahrtr reviewed Apr 23, 2024
View reviewed changes
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
cmd/bbolt/command_check.go Outdated Show resolved Hide resolved
@Elbehery
add pageId flag to check cmd
acfa086 
Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>
@Elbehery Elbehery force-pushed the add-check-subcommand-pgid branch from 94152c1 to acfa086 Compare April 23, 2024 17:38
ahrtr
ahrtr approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@ahrtr ahrtr 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! @Elbehery

ivanvc
ivanvc approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

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

Thanks @Elbehery. LGTM.

@ahrtr ahrtr mentioned this pull request Apr 23, 2024
Closed
@ahrtr ahrtr merged commit 2112e9c into etcd-io:main Apr 23, 2024
16 checks passed
@ahrtr ahrtr changed the title feat: add page-Id flag to check cmd feat: add --from-page flag to check cmd May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjungblu tjungblu tjungblu left review comments

@ahrtr ahrtr ahrtr approved these changes

@ivanvc ivanvc ivanvc approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Elbehery @ahrtr @ivanvc @tjungblu