-
Notifications
You must be signed in to change notification settings - Fork 3
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
Inline method Improvement #54
Conversation
Could you please add test for each fixed bug? |
sure I will |
original_func = dict_original_invocations.get(invocation_node.member)[0] # type: ignore | ||
body_start_line, body_end_line = method_body_lines(original_func, file_path) | ||
text_lines = read_text_with_autodetected_encoding(str(file_path)).split('\n') | ||
|
||
line_to_csv = [ | ||
str(file_path), | ||
file_path, |
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 must be a string, otherwise there will be dirty string like Path("A/b/A") instead of "A/b/A"
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.
У меня кстати не было такого
@@ -267,7 +264,7 @@ def insert_code_with_new_file_creation( | |||
return line_to_csv | |||
|
|||
|
|||
def analyze_file(file_path: Path, output_path: Path) -> List[Any]: | |||
def _analyze_file(file_path: Path, output_path: Path) -> List[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.
why do u make _
before each function? It's not a private function
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.
Я удалил потом
|
||
|
||
class InlineWithReturnWithoutArguments(IBaseInlineAlgorithm): | ||
|
||
def __init__(self): | ||
super().__init__() | ||
|
||
def inline_function( | ||
def get_lines_before_invocation( |
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.
You have the same function. I see, we have a code duplication, extract this default functionality to a protected method in the base abstract class
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
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.
Fix
1cdd52b
to
01a0c67
Compare
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.
ok
Fixed in #50