-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix fixall title suffix #86
Fix fixall title suffix #86
Conversation
Setting the unsafe_fixes to true will not change the behaviour of the "Fix all" action, this is actually part of the specification. I don't really have an opinion other than we would be the first to call it "Fix all (safe fixes)", but do we really need this? It is pointed out twice in the README, once in the configuration example and at the end. |
do we really need this?
I think yes, to me I am not the first time confused 'fix all' meaning, specifically when I chose that action, but it would may be actually do nothing (no safe fixed there actually).
…--
shane.xb.qian
|
"Fix all (all safe fixes)"
"Fix all (only safe fixes)"
"Fix all (safe fixes)"
you can choose one.
…--
shane.xb.qian
|
@jhossbach please check again if ok to you. |
so is it ok to merge now?
"""Optional type.
Optional[X] is equivalent to Union[X, None].
""" there is no blank line before the close
but anyway those should be not a big deal, let me know if still need something to modify, or you can help me refine it as well. thanks. |
clear the 'fix all' meaning:
1, actually it is a 'term' which the 'all' meant all safe fixes only.
2, somehow some '(unsafe)' marker e.g F841 maybe would not be added, vs adding 'all safe fixes' suffix to 'fix all' probably would be a more direct meaning title.