-
Notifications
You must be signed in to change notification settings - Fork 544
refactor: use Path objects instead of strings #5142
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
base: main
Are you sure you want to change the base?
Conversation
@terriko what do you think? |
5221840
to
9f5ea16
Compare
@terriko I forgot to run longer tests locally and had to change some things afterwards. |
0108084
to
1fd509e
Compare
I missed one str/Path in one of the test files. It should pass now - three times a charm :) |
1fd509e
to
75bd83b
Compare
I added more small changes in tests which seem to not run on CI. |
@terriko everything has passed except one unrelated test like the last time, maybe we should change/fix it in another PR? |
Pretty sure the last test was fixed with #4929 so it can be ignored! |
Great news! Shall I update my branch and then rerun CI? |
@terriko do you have any objections? |
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.
Just a heads up: this is blocked because everything is blocked right now, but I'm marking it as approved so that I remember it should be good to merge when we get our CI problem resolved. Nothing new for you to do at this point!
Don't convert Paths to Paths. Simplify code and take advantage of Path object methods. Signed-off-by: Rafał Ilnicki <[email protected]>
75bd83b
to
a8e3d64
Compare
@terriko I needed to resolve a merge conflict in |
Don't convert Paths to Paths.
Simplify code and take advantage of Path object methods.