-
Notifications
You must be signed in to change notification settings - Fork 9
Adds KdfResponses component #571
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
Conversation
|
Great idea, just took a quick look at the PR and remembered this in KDF https://github.com/KomodoPlatform/komodo-defi-framework/blob/main/mm2src/mm2_test_helpers/src/for_tests.rs |
shamardy
left a comment
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.
First review!
| self.valid_response_types = {'success', 'error'} | ||
|
|
||
| # Valid request key pattern (alphanumeric, camelCase/PascalCase) | ||
| self.request_key_pattern = re.compile(r'^[A-Z][a-zA-Z0-9]*$') |
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.
nit: This doesn't allow camelCase
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.
Fixed in cc71c88
Everything is PascalCase so dropped the camel, but made provision for numeric entry (e.g. 1InchMethods)
| parser.add_argument('--fix-minor-issues', action='store_true', | ||
| help='Automatically fix minor formatting issues') |
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.
Is there any usage of fix-minor-issues arg / flag? I can't find any
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.
nope looks like a remnant. removed in e8f3759
| f"Error reading file: {str(e)}" | ||
| )) | ||
|
|
||
| def _validate_file_structure(self, file_path: Path, data: Dict, version: str): |
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.
version is never used
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, removed in acfdaed
| "platform_coin": { | ||
| "platform": "ETH", | ||
| "ticker": "MATIC" | ||
| }, |
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.
Is the response returned like this, I didn't look at kdf but I think you duplicated ticker here
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.
Responses will be pruned and live-generated once #582 is wired up.
| "title": "Transport error", | ||
| "notes": "Connection to ETH node failed", | ||
| "json": { | ||
| "error": "Transport error: JsonRpcError { client_info: \"coin: ETH\", request: JsonRpcRequest { jsonrpc: \"2.0\", id: \"9\", method: \"eth_getBalance\", params: [String(\"0x7Bc1bBDD6A0a722fC9bffc49c921B685ECB84b94\"), String(\"latest\")] }, error: Transport(\"All electrum servers unavailable!\") }" |
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.
Enable eth shouldn't have All electrum servers unavailable in errors :)
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.
Responses will be pruned and live-generated once #582 is wired up.
…nks, formatted md content, checked presence of file and dirs based on sidebar, checked presence of h1 in every file, update Komodo DeFi Framework methods table, adds/updates preview images when base is main
…nks, formatted md content, checked presence of file and dirs based on sidebar, checked presence of h1 in every file, update Komodo DeFi Framework methods table, adds/updates preview images when base is main
…rm/komodo-docs-mdx into responses-component
…nks, formatted md content, checked presence of file and dirs based on sidebar, checked presence of h1 in every file, update Komodo DeFi Framework methods table, adds/updates preview images when base is main
I'm happy to follow whichever structure is most convenient for KDF team (and if required, make changes to mirror the same in docs repo). |
Pairs with https://github.com/gcharang/komodo-docs-revamp-2023/pull/194