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

Add log #624

Merged
merged 3 commits into from
Dec 1, 2023
Merged

Add log #624

merged 3 commits into from
Dec 1, 2023

Conversation

Elbehery
Copy link
Member

This PR add logger to bbolt, see https://github.com/etcd-io/raft/blob/main/logger.go

Fixes #509

This PR is a continuation of #513

cc @ahrtr @fuweid

@CaojiamingAlan @Elbehery
add logger
7d93161 
Signed-off-by: caojiamingalan <[email protected]>
@Elbehery Elbehery mentioned this pull request Nov 27, 2023
Closed
CaojiamingAlan and others added 2 commits November 27, 2023 17:26
@CaojiamingAlan @Elbehery
add tx.ID() in log. Change the position of log. If we call tx.ID() be…
5ce15f0 
…fore testing the nullity of tx.db, it may panic.

Signed-off-by: caojiamingalan <[email protected]>
@Elbehery
logger: add logger to bbolt
9e8668e 
Signed-off-by: Mustafa Elbehery <[email protected]>
@Elbehery
Copy link
Member Author

@ahrtr had to rebase because the last commit i cherrypicked had no sign-off :/

ahrtr
ahrtr approved these changes Nov 27, 2023
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

cc @serathius @fuweid @tjungblu

@ahrtr ahrtr mentioned this pull request Nov 27, 2023
Closed
@tjungblu
Copy link
Contributor

lgtm, generally. Why are we rolling our own logger based on go/log btw?

@Elbehery
Copy link
Member Author

Thanks @tjungblu

We can use diff logger, but we follow the same as in etcd

@ahrtr
Copy link
Member

ahrtr commented Nov 28, 2023

Why are we rolling our own logger based on go/log btw?

Please read conversation in #509, especially #509 (comment)

fuweid
fuweid approved these changes Nov 29, 2023
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@Elbehery
Copy link
Member Author

can we merge this ?

@fuweid
Copy link
Member

fuweid commented Dec 1, 2023

ping @ahrtr @serathius

@ahrtr ahrtr merged commit 125523a into etcd-io:master Dec 1, 2023
13 checks passed
@Elbehery Elbehery deleted the add_log branch January 12, 2024 20:43
@Elbehery Elbehery mentioned this pull request Feb 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fuweid fuweid fuweid approved these changes

@ahrtr ahrtr ahrtr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add logger to bbolt
5 participants
@Elbehery @tjungblu @ahrtr @fuweid @CaojiamingAlan