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

fedekunze/1648 gov params tab #1678

Merged
merged 23 commits into from
Dec 4, 2018
Merged

fedekunze/1648 gov params tab #1678

merged 23 commits into from
Dec 4, 2018

Conversation

fedekunze
Copy link
Contributor

@fedekunze fedekunze commented Nov 30, 2018

Closes #1648

Description:

❤️ Thank you!


  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #1678 into develop will increase coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1678      +/-   ##
===========================================
+ Coverage     97.7%   97.72%   +0.01%     
===========================================
  Files          101      102       +1     
  Lines         2052     2062      +10     
  Branches        92       92              
===========================================
+ Hits          2005     2015      +10     
  Misses          36       36              
  Partials        11       11
Impacted Files Coverage Δ
.../src/renderer/components/staking/TabParameters.vue 100% <ø> (ø) ⬆️
app/src/renderer/routes.js 100% <ø> (ø) ⬆️
.../renderer/components/governance/PageGovernance.vue 100% <ø> (ø) ⬆️
...src/renderer/vuex/modules/governance/parameters.js 100% <100%> (ø) ⬆️
app/src/renderer/connectors/lcdClientMock.js 99.19% <100%> (ø) ⬆️
...c/renderer/components/governance/TabParameters.vue 100% <100%> (ø)
app/src/renderer/vuex/getters.js 94.59% <100%> (+0.15%) ⬆️

@fedekunze fedekunze changed the title WIP: fedekunze/1648 gov params tab fedekunze/1648 gov params tab Dec 3, 2018
@fedekunze fedekunze requested a review from sabau December 3, 2018 16:52
@luniehq luniehq deleted a comment from codecov bot Dec 3, 2018
jbibla
jbibla previously requested changes Dec 3, 2018
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
app/src/renderer/components/governance/TabParameters.vue Outdated Show resolved Hide resolved
Loose {{ parameters.parameters.bond_denom }}
Loose
{{
stakingParameters.parameters.bond_denom
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this logic necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah if there's an error to display the params then the title won't be displayed completely

Copy link
Collaborator

Choose a reason for hiding this comment

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

when would it be better to show bondingDenom over stakingParameters.parameters.bond_denom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

idk, is there a case on which the bondingDenom is not set? maybe we should check that and remove the hardcoded values from parameters everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(on another PR)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes! can you create an issue?

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@jbibla jbibla merged commit 1ceb282 into develop Dec 4, 2018
@fedekunze fedekunze deleted the fedekunze/1648-gov-params-tab branch December 4, 2018 18:56
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.

Implement Governance Proposals Tab
3 participants