Group tags by era (Shelley vs Byron) in swagger documentation #841
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KtorZ
force-pushed
the
KtorZ/review-swagger-hierarchy
branch
3 times, most recently
from
95d8c3d
to
606e03d
Compare
piotr-iohk
reviewed
Oct 16, 2019
specifications/api/swagger.yaml
Outdated
| get: | ||
| operationId: listByronWallets | ||
| tags: ["Byron Wallets"] | ||
| summary: List | ||
| description: | | ||
| <p align="right">status: <strong>not implemented</strong></p> | ||
| <p align="right">status: <strong>provisional</strong></p> |
There was a problem hiding this comment.
as discussed... under testing sounds better...
KtorZ
force-pushed
the
KtorZ/review-swagger-hierarchy
branch
from
606e03d
to
06cd335
Compare
KtorZ
changed the base branch from
jonathanknowles/migrate-byron-wallet-return-type
to
master
|
bors try |
|
bors r+ |
tryBuild failed |
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 16, 2019
841: Group tags by era (Shelley vs Byron) in swagger documentation r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #774 #775 #776 #777 #778 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have renamed `/external-transactions` to `/proxy/transactions` to better convey its meaning - [x] I have renamed all `/byron/wallets` to `/byron-wallets` as per the discussion we had on slack - [x] I have renamed `/.../migrate` to `/.../migrations`, REST is about resources, not actions. - [x] I have grouped and organize API tags by eras such that it's more readable:  # Comments <!-- Additional comments or screenshots to attach if any --> :warning: base branch is `jonathanknowles/migrate-byron-wallet-return-type` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
Build failed |
|
The
but it's a bit far-fetched (and would cause a pretty big changes for all other endpoints to keep the consistency). Going further with RESTful practices, I recommend reading: https://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api#restful |
|
bors try |
tryBuild failed |
…lley) Also renamed '/byron/wallets' to '/byron-wallets' to better reflect what the resource should be
KtorZ
force-pushed
the
KtorZ/review-swagger-hierarchy
branch
from
06cd335
to
d9bcf54
Compare
|
Fixed tests and re-run integration tests locally 👍, they pass. |
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 16, 2019
841: Group tags by era (Shelley vs Byron) in swagger documentation r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #774 #775 #776 #777 #778 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have renamed `/external-transactions` to `/proxy/transactions` to better convey its meaning - [x] I have renamed all `/byron/wallets` to `/byron-wallets` as per the discussion we had on slack - [x] I have renamed `/.../migrate` to `/.../migrations`, REST is about resources, not actions. - [x] I have grouped and organize API tags by eras such that it's more readable:  # Comments <!-- Additional comments or screenshots to attach if any --> :warning: base branch is `jonathanknowles/migrate-byron-wallet-return-type` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
Build succeeded |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
#774 #775 #776 #777 #778
Overview
/external-transactionsto/proxy/transactionsto better convey its meaning/byron/walletsto/byron-walletsas per the discussion we had on slack/.../migrateto/.../migrations, REST is about resources, not actions.Comments
jonathanknowles/migrate-byron-wallet-return-type