Skip to content

MVP#1

Open
Nigma-Ks wants to merge 45 commits intomvpfrom
run-runstrat-in-docker
Open

MVP#1
Nigma-Ks wants to merge 45 commits intomvpfrom
run-runstrat-in-docker

Conversation

@Nigma-Ks
Copy link
Collaborator

@Nigma-Ks Nigma-Ks commented Mar 5, 2026

No description provided.

@Nigma-Ks Nigma-Ks requested a review from gsvgit March 5, 2026 22:18
@gsvgit gsvgit requested a review from Anya497 March 6, 2026 05:52
Copy link

@Anya497 Anya497 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool! Just a few refactor comments.



def get_thread_filepath(uid, filepath):
base_prefix = TMP_FILE_DIR + "/"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use os.sep here.

writer.writerow(["dll", "method"])

for item in parsed_methods:
if "," in item:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is there no comma in the item? Does it really not need processing?


result = Methods.parse_frontend_file_to_dll_methods(temp_file.name)

expected = defaultdict(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't understand why defaultdict is used here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple duplicate strings with paths that can be moved into a separate variable.

dll_methods = defaultdict(list)
for node in selection_tree:
dll_name = node["title"] + ".dll"
dll_methods[dll_name] = []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to explicitly specify the list using defaultdict?

):
with open(front_selection_resource_path, "r") as f:
content = f.read()
prefix = "export const METHODS = "
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move prefix to constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants