- Notifications
You must be signed in to change notification settings - Fork2.5k
zap-api-scan.py: refactoring attempt#6410
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| sys.exit(3) | ||
| ifformat!='openapi'andformat!='soap'andformat!='graphql': | ||
| logging.warning('Format must beeither\'openapi\',\'soap\', or\'graphql\'') | ||
| ifformatnotinSUPPORTED_FORMATS: |
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.
here and log levels can be validated by switching fromgetopt ->ArgumentParser. In python2, Argument parser was not in the PSL, while in python3 you have it already and you don't need to install it anymore.
| port=get_free_port() | ||
| logging.debug('Using port:'+str(port)) | ||
| logging.debug('Using port:%s',port) |
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.
the logger takes care callingstr orrepr depending on%s or%r
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.
Any reason why not using f-string syntax ?
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.
I used to create strings only if logs are actually printed. Probably in this case it is ok using f-string.
| f.write('# You can add your own messages to each rule by appending them after a tab on each line.\n') | ||
| forkey,ruleinsorted(all_dict.items()): | ||
| f.write(key+'\tWARN\t('+rule+')\n') | ||
| f.write(f'{key}\tWARN\t({rule})\n') |
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.
python 3.6 will be out of support from 2021-12. If we want to retain it, we can't use f-strings. Have your say.
thc202 commentedFeb 5, 2021
It might not worth doing major "cosmetic" refactors as per#6406 (comment) but if you still think worth doing to make other changes you plan easier, I'm all for it. |
This PR
will be useful to discuss some refactoring proposal, including:
%ror%swith logging instead of string concatenation for code readability;have your say.