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

refactor: Remove unnecessary code from proto/varlogpb/log_entry.go #927

Merged

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Dec 1, 2024

What this PR does

  • replaced InvalidLogEntry with an empty LogEntry across the codebase
  • removed proto/varlogpb/log_entry.go as it contained unused code
  • updated related files to reflect these changes

@ijsong ijsong requested a review from hungryjang as a code owner December 1, 2024 16:20
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.09%. Comparing base (2c3818a) to head (78f315c).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #927   +/-   ##
=======================================
  Coverage   79.08%   79.09%           
=======================================
  Files         180      179    -1     
  Lines       21615    21607    -8     
=======================================
- Hits        17094    17089    -5     
+ Misses       3711     3695   -16     
- Partials      810      823   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ijsong ijsong force-pushed the tests_for_syncposition_and_syncrange branch from 4499f90 to 9ab27cb Compare December 13, 2024 01:53
@ijsong ijsong force-pushed the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch from 382277e to a62e2db Compare December 13, 2024 01:53
@ijsong ijsong force-pushed the tests_for_syncposition_and_syncrange branch from 9ab27cb to c734a48 Compare December 13, 2024 02:01
@ijsong ijsong force-pushed the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch from a62e2db to 1001a00 Compare December 13, 2024 02:01
@ijsong ijsong force-pushed the tests_for_syncposition_and_syncrange branch from c734a48 to 21de96f Compare January 16, 2025 04:19
@ijsong ijsong force-pushed the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch from 1001a00 to eceacfe Compare January 16, 2025 04:19
@ijsong ijsong requested a review from hungryjang February 2, 2025 10:07
@ijsong ijsong force-pushed the tests_for_syncposition_and_syncrange branch from 21de96f to 954578f Compare February 3, 2025 10:57
@ijsong ijsong force-pushed the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch from eceacfe to fc9386d Compare February 3, 2025 10:57
- Add tests for StorageNodeMetadataDescriptor and LogStreamReplicaMetadataDescriptor
- Test methods: ToStorageNodeDescriptor, GetLogStream, Head, and Tail
- Ensure proper coverage for edge cases and nil values
- Add comments for SyncPosition.InvalidSyncPosition, SyncPosition.Invalid,
  SyncPosition.LessThan, SyncRange.InvalidSyncRange, and SyncRange.Validate
  methods.
- Add unit tests for SyncRange validation and invalid cases.
- replaced InvalidLogEntry with an empty LogEntry across the codebase
- removed proto/varlogpb/log_entry.go as it contained unused code
- updated related files to reflect these changes
@ijsong ijsong force-pushed the tests_for_syncposition_and_syncrange branch from 954578f to a037220 Compare February 3, 2025 11:24
@ijsong ijsong force-pushed the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch from fc9386d to 78f315c Compare February 3, 2025 11:24
Base automatically changed from tests_for_syncposition_and_syncrange to main February 3, 2025 11:46
@ijsong ijsong merged commit cee81ca into main Feb 3, 2025
10 checks passed
@ijsong ijsong deleted the refactor_remove_unnecessary_code_from_proto_varlogpb_log_entry branch February 3, 2025 11:46
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.

2 participants