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

feat(feature-activation): new MUST_SIGNAL and LOCKED_IN states #675

Merged
merged 2 commits into from
Jul 10, 2023

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Jun 27, 2023

Implementation of the design in HathorNetwork/rfcs#58

Acceptance Criteria

  • Implement new MUST_SIGNAL and LOCKED_IN states
  • Rename feature criteria's attribute from activate_on_timeout to lock_in_on_timeout
  • Remove the minimum_activation_height rule that it couldn't be greater than the timeout_height
  • Update timeout_height rule so it must be at least two evaluation intervals after start_height
  • Update existing tests and add new ones

@glevco glevco self-assigned this Jun 27, 2023
@glevco glevco force-pushed the feat/feature-activation/new-states branch from 2c7f504 to a515fb6 Compare June 28, 2023 17:04
@glevco glevco marked this pull request as ready for review June 28, 2023 17:04
@glevco glevco requested review from jansegre and msbrogli as code owners June 28, 2023 17:04
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #675 (a515fb6) into master (e486924) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head a515fb6 differs from pull request most recent head a91c88c. Consider uploading reports for the commit a91c88c to get more accurate results

@@            Coverage Diff             @@
##           master     #675      +/-   ##
==========================================
- Coverage   84.13%   84.12%   -0.01%     
==========================================
  Files         246      246              
  Lines       20442    20446       +4     
  Branches     2757     2759       +2     
==========================================
+ Hits        17198    17201       +3     
- Misses       2672     2673       +1     
  Partials      572      572              
Impacted Files Coverage Δ
hathor/feature_activation/feature_service.py 97.46% <100.00%> (+0.24%) ⬆️
hathor/feature_activation/model/criteria.py 100.00% <100.00%> (ø)
hathor/feature_activation/model/feature_state.py 100.00% <100.00%> (ø)
hathor/feature_activation/resources/feature.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

jansegre
jansegre previously approved these changes Jun 29, 2023
msbrogli
msbrogli previously approved these changes Jul 7, 2023
@glevco glevco dismissed stale reviews from msbrogli and jansegre via a91c88c July 7, 2023 21:07
@glevco glevco force-pushed the feat/feature-activation/new-states branch from a515fb6 to a91c88c Compare July 7, 2023 21:07
@glevco glevco requested review from jansegre and msbrogli July 7, 2023 21:09
@jansegre jansegre merged commit c84ca8b into master Jul 10, 2023
@jansegre jansegre deleted the feat/feature-activation/new-states branch July 10, 2023 15:51
@jansegre jansegre mentioned this pull request Jul 12, 2023
2 tasks
This was referenced Aug 2, 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