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 li proposal test #2074

Merged
merged 10 commits into from
Feb 28, 2019
Merged

refactor li proposal test #2074

merged 10 commits into from
Feb 28, 2019

Conversation

jbibla
Copy link
Collaborator

@jbibla jbibla commented Feb 24, 2019

  • new test stylez for li proposal test

For contributor:

  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer
  • Attach screenshots of the UI components on the PR description (if applicable)
  • Scope of work approved for big PRs

For reviewer:

  • Manually tested the changes on the UI

sabau
sabau previously requested changes Feb 26, 2019
Copy link
Contributor

@sabau sabau left a comment

Choose a reason for hiding this comment

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

Unfortunately there is a lot of duplicated code between PageProposal and LiProposal we should kill the duplication and put it in some helper, tested separately, and I guess the tests of this component will all die, along with some tests from PageProposal

CHANGELOG.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 26, 2019

Codecov Report

Merging #2074 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop   #2074   +/-   ##
=======================================
  Coverage     95.4%   95.4%           
=======================================
  Files          110     110           
  Lines         2570    2570           
  Branches       103     103           
=======================================
  Hits          2452    2452           
  Misses         109     109           
  Partials         9       9
Impacted Files Coverage Δ
.../src/renderer/components/governance/LiProposal.vue 100% <ø> (ø) ⬆️

@codecov
Copy link

codecov bot commented Feb 26, 2019

Codecov Report

Merging #2074 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2074   +/-   ##
========================================
  Coverage    95.31%   95.31%           
========================================
  Files          110      110           
  Lines         2581     2581           
  Branches       104      104           
========================================
  Hits          2460     2460           
  Misses         112      112           
  Partials         9        9
Impacted Files Coverage Δ
...pp/src/renderer/components/staking/LiValidator.vue 96% <ø> (ø) ⬆️
.../src/renderer/components/governance/LiProposal.vue 100% <ø> (ø) ⬆️

@fedekunze fedekunze requested a review from sabau February 26, 2019 17:30
@sabau sabau dismissed their stale review February 26, 2019 17:58

Maybe we can fix the duplicated code separately, so we can clean up the PR queue :)

CHANGELOG.md Outdated
@@ -47,6 +51,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Fixed

- Refactored tests: PageStaking, PanelSort, TabStakingParameters @faboweb

## Fixed
Copy link
Contributor

Choose a reason for hiding this comment

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

dup fixed entry

@@ -31,16 +31,16 @@
</td>
<td>{{ `#` + proposal.proposal_id }}</td>
<td class="li-proposal__value yes">
{{ tally.yes }}
{{ tally.yes || `--` }}
Copy link
Contributor

Choose a reason for hiding this comment

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

side note: we should maybe prettify the tally result numbers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what do you mean by prettify?

Copy link
Collaborator

Choose a reason for hiding this comment

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

there is a function called pretty which shortens the value after 4 decimals

@faboweb faboweb merged commit f8fc804 into develop Feb 28, 2019
@faboweb faboweb deleted the jordan/1337-li-proposal-tests branch February 28, 2019 09:35
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.

4 participants