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(app): add monitoringp module #721

Merged
merged 7 commits into from
Apr 15, 2022
Merged

feat(app): add monitoringp module #721

merged 7 commits into from
Apr 15, 2022

Conversation

aljo242
Copy link
Contributor

@aljo242 aljo242 commented Apr 13, 2022

Closes #715

  • Set lastBlockHeight to 1 instead of 0. This still disables any monitoringp module actions

@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #721 (b8b8527) into develop (d35cf06) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #721   +/-   ##
========================================
  Coverage    11.19%   11.19%           
========================================
  Files          278      278           
  Lines        60780    60780           
========================================
+ Hits          6804     6805    +1     
+ Misses       53792    53791    -1     
  Partials       184      184           
Impacted Files Coverage Δ
x/monitoringp/types/params.go 68.42% <100.00%> (ø)
x/campaign/simulation/store.go 83.10% <0.00%> (+0.67%) ⬆️

giunatale
giunatale previously approved these changes Apr 13, 2022
Copy link
Contributor

@giunatale giunatale left a comment

Choose a reason for hiding this comment

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

out of curiosity, why 1 and not 0?

@aljo242
Copy link
Contributor Author

aljo242 commented Apr 13, 2022

out of curiosity, why 1 and not 0?

Technically, block heights can't even be 0. 1 is the minimum. We also have parameter validation checking if the lastBlockHeight is GTE 1.

@lumtis
Copy link
Contributor

lumtis commented Apr 13, 2022

Technically, block heights can't even be 0. 1 is the minimum. We also have parameter validation checking if the lastBlockHeight is GTE 1.

Yes you're right, 1 will do nothing as well

@lumtis
Copy link
Contributor

lumtis commented Apr 13, 2022

Let me just some time for the review, I will have to test the incentivized testnet workflow

lumtis
lumtis previously approved these changes Apr 14, 2022
@aljo242 aljo242 dismissed stale reviews from lumtis and giunatale via 0473124 April 14, 2022 16:36
Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>
@aljo242 aljo242 requested a review from giunatale April 14, 2022 16:36
Copy link
Contributor

@giunatale giunatale left a comment

Choose a reason for hiding this comment

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

👍

@giunatale
Copy link
Contributor

failure in simtests seem to be related to #725

@giunatale giunatale merged commit 63dff9d into develop Apr 15, 2022
@giunatale giunatale deleted the feat/add-monitoringp branch April 15, 2022 14:03
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.

Import monitoring-provider in app.go
3 participants