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

Various improvements #37

Merged
merged 9 commits into from Nov 8, 2019
Merged

Various improvements #37

merged 9 commits into from Nov 8, 2019

Conversation

RiccardoM
Copy link
Contributor

Description

Checklist

  • Targeted PR against correct branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Re-reviewed Files changed in the Github PR explorer

- Improved the posts events
- Replaced all the time.Time instances with block heights (works towards #23)
- Replaced the creation time with block height
@RiccardoM RiccardoM added kind/enhancement Enhance an already existing feature; no "New feature" to add quality/code-cleanliness Improves the overall quality of code R4R labels Nov 6, 2019
@codecov-io
Copy link

codecov-io commented Nov 6, 2019

Codecov Report

Merging #37 into develop will increase coverage by 19.54%.
The diff coverage is 65.51%.

Impacted file tree graph

@@             Coverage Diff              @@
##           develop      #37       +/-   ##
============================================
+ Coverage    41.64%   61.18%   +19.54%     
============================================
  Files            4       13        +9     
  Lines          341      541      +200     
============================================
+ Hits           142      331      +189     
+ Misses         199      192        -7     
- Partials         0       18       +18
Impacted Files Coverage Δ
x/magpie/internal/types/codec.go 100% <ø> (ø)
x/posts/internal/keeper/querier.go 0% <0%> (ø)
x/posts/internal/types/like.go 0% <0%> (ø)
x/magpie/internal/types/session.go 0% <0%> (ø)
x/posts/internal/types/codec.go 100% <100%> (ø)
x/magpie/internal/types/msgs.go 34.78% <20%> (ø)
x/posts/internal/types/msgs.go 43.83% <43.83%> (ø)
x/posts/internal/types/post.go 5.88% <7.14%> (ø)
x/magpie/internal/keeper/querier.go 66.66% <83.33%> (+66.66%) ⬆️
x/posts/internal/keeper/keeper.go 89.53% <89.53%> (ø)
... and 14 more

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 81897ba...3b4ec6c. Read the comment docs.

Expiry: msg.Created.Add(time.Minute * 14400),
SessionID: keeper.GetLastSessionID(ctx).Next(),
Created: ctx.BlockHeight(),
Expiry: ctx.BlockHeight() + 240, // 24 hours, counting a 6 secs block interval
Copy link
Contributor

@kwunyeung kwunyeung Nov 7, 2019

Choose a reason for hiding this comment

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

Nice one! May put this in genesis params later.

Copy link
Contributor Author

@RiccardoM RiccardoM Nov 8, 2019

Choose a reason for hiding this comment

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

Opened #38 to have a better discussion there.

case types.MsgLikePost:
return handleMsgLike(ctx, keeper, msg)
default:
errMsg := fmt.Sprintf("Unrecognized Magpie Msg type: %v", msg.Type())
Copy link
Contributor

@kwunyeung kwunyeung Nov 7, 2019

Choose a reason for hiding this comment

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

This error message should be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

abci "github.com/tendermint/tendermint/abci/types"
)

// query endpoints supported by the magpie Querier
const (
QueryPost = "post"
QueryLike = "like"
QuerySession = "testSession"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks this would give error when query sessions/ endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks

Copy link
Contributor

@kwunyeung kwunyeung left a comment

Choose a reason for hiding this comment

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

Some minor change request and questions. Tested the commands locally.

@kwunyeung kwunyeung merged commit e0ee3a1 into develop Nov 8, 2019
@RiccardoM RiccardoM deleted the riccardo/module-separation branch November 11, 2019 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add quality/code-cleanliness Improves the overall quality of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split the magpie module Magpie Session created and expiration should be replaced with block height
3 participants