Updated file paths to use pathlib.Path #1583
Conversation
|
Some changes are still left, will do them and give updates soon |
Aayush-Goel-04
force-pushed
the
Aayush-Goel-04/Issue#1534
branch
2 times, most recently
from
ab833af
to
6218f31
Compare
There was a problem hiding this comment.
this PR is looking quite good. thank you @Aayush-Goel-04 for going through the codebase so thoroughly to update the use of path manipulation routines.
i have a few inline comments. i'll tag some of them 👀 for things i think should be addressed before we merge. other things are more questions/discussions and can optionally be addressed.
in general, i'd request that you revert changes to the capitalization of comments and logging messages. its true that the existing grammar is not good, since many sentences should be capitalized; however, even more important right now is consistency, so we should address the grammar separately and thoroughly. thanks for trying to improve this in a few places, though.
|
Will work on converting args.sample type from str to pathlib.Path across capa. |
capa/main.py
Outdated
| @@ -500,7 +501,7 @@ def get_workspace(path, format_, sigpaths): | |||
| else: | |||
| raise ValueError("unexpected format: " + format_) | |||
|
|
|||
| viv_utils.flirt.register_flirt_signature_analyzers(vw, sigpaths) | |||
| viv_utils.flirt.register_flirt_signature_analyzers(vw, [s.as_posix() for s in sigpaths]) | |||
There was a problem hiding this comment.
better str vs as_posix here?
There was a problem hiding this comment.
Will change to str representations as it will be used in runtime. Later when changes are made in viv utils package it will be changed to Path types
scripts/capa_as_library.py
Outdated
|
|
||
| parser = argparse.ArgumentParser(description="Extract capabilities from a file") | ||
| parser.add_argument("file", help="file to extract capabilities from") | ||
| parser.add_argument("--rules", help="path to rules directory", default=os.path.abspath(RULES_PATH)) | ||
| parser.add_argument("--rules", help="path to rules directory", default=RULES_PATH) |
There was a problem hiding this comment.
does this use the string in the help output then successfully?
There was a problem hiding this comment.
If no value is given then its default is RULES_PATH of type Path . else it is converted to Path later
There was a problem hiding this comment.
looking great! a few more instances to change over (i think, happy to discuss) and then im excited to merge. thanks for the great work @Aayush-Goel-04
|
now that #1591 is merged, lets also enable flake8-use-pathlib and ensure it passes in this PR. https://gitlab.com/RoPP/flake8-use-pathlib this should just involve adding this will close #1599 |
Aayush-Goel-04
force-pushed
the
Aayush-Goel-04/Issue#1534
branch
2 times, most recently
from
d1a1c68
to
b84af6a
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
closes #1534
Checklist