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(sync): remove legacy sync-v1.0 #864

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

jansegre
Copy link
Member

@jansegre jansegre commented Nov 9, 2023

Motivation

It's been legacy for a while and we haven't even deployed any nodes with 1.0 support for a while.

Acceptance Criteria

  • v1_1 references become v1, and v1 is removed
  • sync-v1 being enabled means SyncVersion.V1_1(with value 'v1.1') is used and never SyncVersion.V1 (with value 'v1') which could previously be used depending on a cli argument
  • the --x-enable-legacy-sync-v1_0 argument is removed, nothing else changes for the CLI

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@jansegre jansegre requested a review from glevco November 9, 2023 23:58
@jansegre jansegre self-assigned this Nov 9, 2023
@jansegre jansegre requested a review from msbrogli as a code owner November 9, 2023 23:58
@jansegre jansegre force-pushed the refactor/remove-legacy-sync-1.0 branch from f365880 to 50edd9e Compare November 10, 2023 00:15
@jansegre jansegre force-pushed the refactor/remove-legacy-sync-1.0 branch from 50edd9e to 07f34ee Compare November 10, 2023 00:21
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (382cc38) 85.42% compared to head (07f34ee) 85.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
- Coverage   85.42%   85.38%   -0.05%     
==========================================
  Files         282      281       -1     
  Lines       22270    22231      -39     
  Branches     3367     3364       -3     
==========================================
- Hits        19025    18981      -44     
  Misses       2583     2583              
- Partials      662      667       +5     
Files Coverage Δ
hathor/builder/builder.py 92.02% <100.00%> (+0.43%) ⬆️
hathor/builder/cli_builder.py 75.00% <100.00%> (-0.13%) ⬇️
hathor/p2p/sync_v1/factory.py 100.00% <ø> (ø)
hathor/p2p/sync_version.py 88.88% <100.00%> (-1.59%) ⬇️
hathor/p2p/states/hello.py 76.23% <0.00%> (+3.96%) ⬆️
hathor/p2p/manager.py 71.39% <50.00%> (-1.19%) ⬇️

... and 8 files with indirect coverage changes

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

@jansegre jansegre merged commit 1171e10 into master Nov 10, 2023
9 checks passed
@jansegre jansegre deleted the refactor/remove-legacy-sync-1.0 branch November 10, 2023 15:01
@jansegre jansegre mentioned this pull request Nov 13, 2023
2 tasks
This was referenced Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants