-
Notifications
You must be signed in to change notification settings - Fork 56
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
swagger.yaml edits for liquidity module #356
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #356 +/- ##
========================================
Coverage 83.26% 83.26%
========================================
Files 20 20
Lines 2247 2247
========================================
Hits 1871 1871
Misses 207 207
Partials 169 169 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the question I commented if we should end sentence with a dot for the title section.
@@ -32,7 +32,7 @@ paths: | |||
The id of the pool_type to use as pool_type_id for | |||
pool creation. | |||
|
|||
Only pool-type-id 1 is supported | |||
Only pool-type-id 1 is supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the sentence should end with a dot, then we should maybe put a dot in the other sentences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for noticing @kogisin
- for title, the style is not to use punctuation (dot) at the end of the sentence
(our titles are probably longer than appropriate, I need to learn more) - for description, we do need punctuation at the end of complete sentences
@kogisin can you take a look at the minor changes I just introduced, so make sure all is accurate and as expected? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
swagger.yaml edits for liquidity module
question: do we want a message description for block heights to be "is" or "was"
in this PR, I am suggesting to use present tense (although I was tempted to use the past tense, I don't know enough yet to make a firm recommendation)
I think that we are recording the block height "when this message was appended" ??
closes: part of #217
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes