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.

Sign up for GitHub

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

Jump to bottom

Turn DisableComments field to enum and make it editable #456

Merged
merged 26 commits into from
Jun 11, 2021
Merged

Turn DisableComments field to enum and make it editable #456

merged 26 commits into from
Jun 11, 2021

Conversation

leobragaz
Copy link
Contributor

@leobragaz leobragaz commented Jun 1, 2021 •
edited
Loading

Description

This PR turn the posts DisableComments field from a bool to an enum type to better handle it and also open the possibility to edit it without any problem.
Closes #445.
Closes #446.

Checklist

  • Targeted PR against correct branch.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote unit tests.
  • Wrote integration tests (simulation & CLI).
  • Updated the documentation.
  • Added an entry to the CHANGELOG.md file.
  • Re-reviewed Files changed in the Github PR explorer.

@leobragaz leobragaz changed the title Turn DisableComments field to enum Turn DisableComments field to enum and make it editable Jun 1, 2021
leobragaz added 2 commits June 1, 2021 17:57
@leobragaz
updated docs
8ba2581 
update CHANGELOG.md
fixed decoder_test.go error
@leobragaz
fixed broken tests
49d1b1e
@codecov
Copy link

codecov bot commented Jun 3, 2021 •
edited
Loading

Codecov Report

Merging #456 (6ad4984) into master (3799b76) will increase coverage by 8.54%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
+ Coverage   75.46%   84.00%   +8.54%     
==========================================
  Files          94       91       -3     
  Lines        4537     4090     -447     
==========================================
+ Hits         3424     3436      +12     
+ Misses        966      505     -461     
- Partials      147      149       +2
Impacted Files Coverage Δ
x/staging/posts/types/reactions.go 95.83% <ø> (ø)
x/staging/posts/keeper/msgs_server.go 95.69% <100.00%> (+0.04%) ⬆️
x/staging/posts/types/msgs.go 96.20% <100.00%> (+0.02%) ⬆️
x/staging/posts/types/posts.go 91.20% <100.00%> (+1.59%) ⬆️
x/staging/posts/keeper/invariants.go 79.36% <0.00%> (-7.94%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3799b76...6ad4984. Read the comment docs.

@leobragaz leobragaz marked this pull request as ready for review June 3, 2021 15:10
@leobragaz leobragaz requested a review from RiccardoM June 3, 2021 15:12
@leobragaz
Copy link
Contributor Author

@RiccardoM please let me know what's lowering the coverage because I can't see anything due to an error 400

@RiccardoM
Copy link
Contributor

@bragaz No idea how, but it's actually decreasing the coverage of 9.62%

@leobragaz
Copy link
Contributor Author

@RiccardoM The coverage errors was caused by uncovered CLI tests due to the staging phase of x/posts.
I've commented them with //DONOTCOVER and I think we can open an issue to remember removing it when we bring them out of staging

@RiccardoM RiccardoM force-pushed the leonardo/disable-comments-to-enum branch from 2045308 to 8d36487 Compare June 10, 2021 05:31
RiccardoM
RiccardoM reviewed Jun 10, 2021
View reviewed changes
Copy link
Contributor

@RiccardoM RiccardoM left a comment

Choose a reason for hiding this comment

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

The implementation looks good. I just have some comments about the namings. Also please @bragaz make sure you pull the changes I've pushed to the branch since I had to force push in order to clear the history and make it linear

CHANGELOG.md Outdated Show resolved Hide resolved
proto/desmos/posts/v1beta1/msgs.proto Show resolved Hide resolved
proto/desmos/posts/v1beta1/posts.proto Outdated Show resolved Hide resolved
x/staging/posts/client/cli/keys.go Outdated Show resolved Hide resolved
leobragaz added 2 commits June 10, 2021 09:08
@leobragaz
Merge branch 'leonardo/disable-comments-to-enum' of https://github.co…
cc2954a 
…m/desmos-labs/desmos into leonardo/disable-comments-to-enum

� Conflicts:
�	x/staging/posts/types/posts.go
@leobragaz
renamed comments_state to comments
733b2f1 
fixed tests and names around the code
@leobragaz leobragaz requested a review from RiccardoM June 10, 2021 08:32
@leobragaz leobragaz requested a review from RiccardoM June 10, 2021 10:02
leobragaz and others added 5 commits June 10, 2021 12:27
@leobragaz
fixed errors in tests
cef254c
@leobragaz
Merge branch 'master' of https://github.com/desmos-labs/desmos into l…
26ca723 
…eonardo/disable-comments-to-enum
@RiccardoM
Reverted "comments" to be "comments state"
599cc01 
This reverts commit 89b3af1
This reverts commit 733b2f1
@RiccardoM
Updated the CHANGELOG
54c7898 
Signed-off-by: Riccardo Montagnin <[email protected]>
@RiccardoM
Fixed tests
6ad4984 
Signed-off-by: Riccardo Montagnin <[email protected]>
@RiccardoM RiccardoM merged commit b0e1021 into master Jun 11, 2021
@RiccardoM RiccardoM deleted the leonardo/disable-comments-to-enum branch June 11, 2021 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RiccardoM RiccardoM RiccardoM approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make the DisableComments field editable Switch DisableComments field to enum type
2 participants
@leobragaz @RiccardoM