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

log: fixup raftLog initialization #151

Merged
merged 3 commits into from
Feb 6, 2024
Merged

Conversation

pav-kv
Copy link
Contributor

@pav-kv pav-kv commented Feb 1, 2024

This PR consolidates the raftLog struct initialization into a single return statement. It also makes all unit-tests use the newLog constructor function to initialize a log, instead of populating the struct manually.

Changes to the unstable / raftLog structures should by default not require fixing boilerplate in tests.

Related to #146
Blocks #139

log.go Outdated Show resolved Hide resolved
@pav-kv
Copy link
Contributor Author

pav-kv commented Feb 5, 2024

Hi @serathius. Could you please merge this and #147? These are pretty trivial test-only clean-ups, but I have other useful work on top of them and would like to avoid the trouble of maintaining these branches or duplicating these cleanups across PRs.

@pav-kv
Copy link
Contributor Author

pav-kv commented Feb 5, 2024

This PR might have a merge conflict with #147. I rebased it on top of #147.

@ahrtr
Copy link
Member

ahrtr commented Feb 6, 2024

Please rebase this PR.

Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
Signed-off-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
@pav-kv
Copy link
Contributor Author

pav-kv commented Feb 6, 2024

@ahrtr done

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

Most change are mechanical changes. cc @erikgrinaker to take a second look

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr ahrtr merged commit 5963780 into etcd-io:main Feb 6, 2024
10 checks passed
@pav-kv pav-kv deleted the raft-log-init branch February 6, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants