Suppress warnings #20
Conversation
Removed warnings reported by PyCharm's Inspect Code, except most spelling reports and some things in README.md.
Codecov Report
@@ Coverage Diff @@
## master #20 +/- ##
==========================================
- Coverage 84.57% 84.25% -0.33%
==========================================
Files 24 24
Lines 525 527 +2
==========================================
Hits 444 444
- Misses 81 83 +2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #20 +/- ##
==========================================
- Coverage 84.57% 84.25% -0.33%
==========================================
Files 24 24
Lines 525 527 +2
==========================================
Hits 444 444
- Misses 81 83 +2
Continue to review full report at Codecov.
|
marcserrat
left a comment
marcserrat
left a comment
There was a problem hiding this comment.
Can you please take a look to my few comments?
README.md
Outdated
| Connect Python SDK allows an easy and fast integration with Connect fulfillment API. Thanks to it you can automate the fulfillment of orders generated by your products. | ||
|
|
||
| In order to use this library, please ensure that you have read first the documentation available on Connect knowladge base article located here, this one will provide you a great information on the rest api that this library implements. | ||
| In order to use this library, please ensure that you have read first the documentation available on Connect knowledge base article located here, this one will provide you a great information on the rest api that this library implements. |
There was a problem hiding this comment.
please, add proper links for "here"
There was a problem hiding this comment.
Since I didn't write the original text, I guess that the most appropriate link for that would the public API documentation? https://confluence.int.zone/display/CONNECT/Connect+Public+API
There was a problem hiding this comment.
Not at all. Please review the original readme of php-sdk, is a copy and paste
README.md
Outdated
| class ExampleRequestProcessor(FulfillmentAutomation): | ||
| def process_request(self, request): | ||
|
|
||
| logger.info('Processing request {}'.format(request.id)) |
There was a problem hiding this comment.
please also include for product X, Contract ID and marketplace
There was a problem hiding this comment.
Fine, will do tomorrow (and from the correct account, I'm on my personal laptop right now ;)).
There was a problem hiding this comment.
Done, I have added contract id and product and marketplace names (since I though it is more meaningful like this):
logger.info('Processing request {} for contract {}, product {}, marketplace {}'
.format(request.id,
request.contract.id,
request.asset.product.name,
request.marketplace.name))
connect/config.py
Outdated
| api_key=None, | ||
| products=None, | ||
| file=None | ||
| filename=None |
There was a problem hiding this comment.
why to rename to filename? People will introduce there config.json, why not file? Shall we differenciate also filepath?
There was a problem hiding this comment.
Because PyCharm's code inspection raises a warning because 'file' is used as a standard type on Python. In this PR, I just made the required changes to the code so it can be inspected by PyCharm without any warnings.
There was a problem hiding this comment.
can you suggest better name than filename? I don't like it 100% becouse people also will put path
5 participants
I have simply made the necessary changes to remove warnings when using PyCharm's Inspect Code tool, except those related to README.md and most spelling errors.
First letter in comments has also been capitalized as indicated by PEP-8 standard.