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

Post ID improvement #132

Merged
merged 23 commits into from
Apr 20, 2020
Merged

Post ID improvement #132

merged 23 commits into from
Apr 20, 2020

Conversation

leobragaz
Copy link
Contributor

@leobragaz leobragaz commented Apr 14, 2020 •
edited
Loading

Description

This PR contains some core changes to the way the Post's IDs are created.
This has been done to optimize the creation and edit of posts.
Closes #131.

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.

Bragaz added 2 commits April 10, 2020 18:04
@leobragaz leobragaz added this to the v0.4.0 milestone Apr 14, 2020
@codecov
Copy link

codecov bot commented Apr 16, 2020 •
edited
Loading

Codecov Report

Merging #132 into master will increase coverage by 0.37%.
The diff coverage is 91.30%.

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
+ Coverage   85.47%   85.84%   +0.37%     
==========================================
  Files          47       50       +3     
  Lines        1996     2120     +124     
==========================================
+ Hits         1706     1820     +114     
- Misses        254      263       +9     
- Partials       36       37       +1

@leobragaz leobragaz marked this pull request as ready for review April 16, 2020 07:58
Bragaz added 2 commits April 16, 2020 16:45
added tests to raise coverage
2443120 
fixed test
fixed docs
added migration
updated CHANGELOG.md
fb24f8f
@leobragaz leobragaz requested a review from RiccardoM April 16, 2020 18:49
@leobragaz leobragaz removed the request for review from RiccardoM April 17, 2020 09:31
leobragaz
leobragaz commented Apr 17, 2020
View reviewed changes
Copy link
Contributor Author

@leobragaz leobragaz left a comment

Choose a reason for hiding this comment

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

re-reviewed

@leobragaz leobragaz requested a review from RiccardoM April 17, 2020 11:35
RiccardoM
RiccardoM suggested changes Apr 17, 2020
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 overall PR looks nice. However there are some things that should be changed IMO.

CHANGELOG.md Outdated Show resolved Hide resolved
docs/developers/queries/poll-answers.md Outdated Show resolved Hide resolved
docs/developers/queries/post.md Outdated Show resolved Hide resolved
docs/developers/queries/posts.md Outdated Show resolved Hide resolved
docs/developers/queries/session.md Outdated Show resolved Hide resolved
x/posts/internal/types/post.go Outdated Show resolved Hide resolved
x/posts/internal/types/querier.go Outdated Show resolved Hide resolved
x/posts/internal/types/post_sorting.go Show resolved Hide resolved
x/posts/internal/keeper/keeper.go Outdated Show resolved Hide resolved
x/posts/internal/keeper/keeper.go Outdated Show resolved Hide resolved
@leobragaz leobragaz requested a review from RiccardoM April 17, 2020 14:33
leobragaz
leobragaz commented Apr 17, 2020
View reviewed changes
Copy link
Contributor Author

@leobragaz leobragaz left a comment

Choose a reason for hiding this comment

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

re-reviewed 2

@leobragaz leobragaz removed the request for review from RiccardoM April 20, 2020 08:36
leobragaz
leobragaz commented Apr 20, 2020
View reviewed changes
Copy link
Contributor Author

@leobragaz leobragaz left a comment

Choose a reason for hiding this comment

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

re-reviewed 3

RiccardoM
RiccardoM approved these changes Apr 20, 2020
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.

LGTM

@RiccardoM RiccardoM merged commit e0326d0 into master Apr 20, 2020
@leobragaz leobragaz deleted the leonardo/postIDs-improvement branch April 20, 2020 13:46
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
x/posts Post module
Projects
None yet
Milestone
v0.4.0
Development

Successfully merging this pull request may close these issues.

Changing the way PostIDs are created
2 participants
@leobragaz @RiccardoM