Skip to content

Convert os.path to pathlib#1547

Closed
SharpEdgeMarshall wants to merge 5 commits intocookiecutter:mainfrom
SharpEdgeMarshall:remove_ospath
Closed

Convert os.path to pathlib#1547
SharpEdgeMarshall wants to merge 5 commits intocookiecutter:mainfrom
SharpEdgeMarshall:remove_ospath

Conversation

@SharpEdgeMarshall
Copy link
Copy Markdown
Contributor

@SharpEdgeMarshall SharpEdgeMarshall commented May 10, 2021 •
edited
Loading

Partially solves #1403

@SharpEdgeMarshall SharpEdgeMarshall marked this pull request as draft May 10, 2021 10:40
@SharpEdgeMarshall SharpEdgeMarshall force-pushed the remove_ospath branch 4 times, most recently from c832e0b to f93ddce Compare May 13, 2021 16:16
@SharpEdgeMarshall SharpEdgeMarshall added enhancement This issue/PR relates to a feature request. WIP Work In Progress labels May 14, 2021
This was linked to issues May 14, 2021
Tests refactoring: writing outside of temp dir #1405
Open
Windows compatibility. Change all os.path invocations to pathlib.Path. #1403
Closed
@SharpEdgeMarshall SharpEdgeMarshall force-pushed the remove_ospath branch 2 times, most recently from 2441dae to 802979a Compare May 21, 2021 14:44
@SharpEdgeMarshall SharpEdgeMarshall force-pushed the remove_ospath branch 2 times, most recently from 209864b to 9f646c6 Compare May 31, 2021 09:26
jensens
jensens reviewed Jun 1, 2022
View reviewed changes
@jensens
Copy link
Copy Markdown
Contributor

jensens commented Jun 1, 2022

Actually I would like to get this one in. But the conflict-merge seems to break tests. Probably I did something wrong. I need to look into it later, or anyone else to step in?

@insspb
Copy link
Copy Markdown
Member

insspb commented Jun 7, 2022

Merged fresh master with updated CI/CD

insspb
insspb reviewed Jun 7, 2022
View reviewed changes
@insspb
Copy link
Copy Markdown
Member

insspb commented Jun 7, 2022

Hmm. Will look on this tomorrow, on windows and locally. Please do not merge before.

@jensens @insspb
use f-string
18f4750 
Co-authored-by: Andrey Shpak <insspb@users.noreply.github.com>
@insspb
Copy link
Copy Markdown
Member

insspb commented Jun 7, 2022

@jensens I really do not like idea to merge it as is. So please do not merge this.

Such big changes I want at least in 2 merge requests. In 1 codebase only. (Tests should be stable, as we do not change app behavior).

In second - tests only.

@jensens
Copy link
Copy Markdown
Contributor

jensens commented Jun 7, 2022

Such big changes I want at least in 2 merge requests. In 1 codebase only. (Tests should be stable, as we do not change app behavior).

In second - tests only.

Indeed, this would be favorable.

@insspb insspb mentioned this pull request Jun 8, 2022
Merged
@kurtmckee
Copy link
Copy Markdown
Member

kurtmckee commented Jun 19, 2023

I'm closing this PR due to its age.

@kurtmckee kurtmckee closed this Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jensens jensens jensens left review comments

+1 more reviewer

@insspb insspb insspb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement This issue/PR relates to a feature request. WIP Work In Progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tests refactoring: writing outside of temp dir Windows compatibility. Change all os.path invocations to pathlib.Path.

4 participants

@SharpEdgeMarshall @jensens @insspb @kurtmckee