NotificationsYou must be signed in to change notification settings
Fork33.3k
Star69.7k

bpo-39950: add`pathlib.Path.hardlink_to()` method that supersedes`link_to()`#18909

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

miss-islington merged 2 commits intopython:masterfrombarneygale:bpo-39925-pathlib-path-hard-link-arg-order

Apr 23, 2021

Merged

bpo-39950: add`pathlib.Path.hardlink_to()` method that supersedes`link_to()`#18909

miss-islington merged 2 commits intopython:masterfrombarneygale:bpo-39925-pathlib-path-hard-link-arg-order

Apr 23, 2021

+64 −4

Conversation

Copy link

Contributor

barneygale commentedMar 11, 2020•
edited by miss-islington
Loading

The argument order oflink_to() is reversed compared to what one may expect, so:

a.link_to(b)

Might be expected to createa as a link tob, in fact it createsb as a link toa, making it function more like a "link from". This doesn't matchsymlink_to() nor the documentation and doesn't seem to be the original author's intent.

This PR deprecateslink_to() and introduceshardlink_to(), which has the same argument order assymlink_to().

https://bugs.python.org/issue39950

Automerge-Triggered-By: GH:brettcannon

the-knights-who-say-ni added the CLA signed label

Mar 11, 2020

bedevere-bot added the awaiting review label

Mar 11, 2020

Copy link

Member

tirkarthi commentedMar 11, 2020

The issue was closed as a duplicate so I would prefer closing the PR. Thanks.

barneygale changed the title~~bpo-39925: addpathlib.Path.hardlink_to() method that supersedeslink_to()~~bpo-39950: addpathlib.Path.hardlink_to() method that supersedeslink_to()

Mar 13, 2020

Copy link

ContributorAuthor

barneygale commentedMar 13, 2020

I've logged a new bug after some discussion on the python-dev mailing list:https://bugs.python.org/issue39950

brettcannon requested changes

Mar 13, 2020

View reviewed changes

Copy link

Member

brettcannon left a comment

Choose a reason for hiding this comment

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

Some doc restructuring and also making sure the public API doesn't break due to this.

Doc/library/pathlib.rst OutdatedShow resolvedHide resolved

Lib/pathlib.py OutdatedShow resolvedHide resolved

bedevere-bot added awaiting changes and removed awaiting review labels

Mar 13, 2020

Copy link

bedevere-bot commentedMar 13, 2020

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

brettcannon requested a review frompitrou

March 13, 2020 19:07

barneygale force-pushed thebpo-39925-pathlib-path-hard-link-arg-order branch from248c6e8 to430b7f7Compare

March 21, 2020 02:09

pitrou reviewed

Apr 17, 2020

View reviewed changes

Copy link

Member

pitrou left a comment

Choose a reason for hiding this comment

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

This looks mostly good. Just two comments.

Also, can you rebase/merge from master?

Lib/test/test_pathlib.py OutdatedShow resolvedHide resolved

Lib/pathlib.py OutdatedShow resolvedHide resolved

barneygale force-pushed thebpo-39925-pathlib-path-hard-link-arg-order branch fromb32aaa6 tob7c8ab2Compare

May 29, 2020 01:27

barneygale force-pushed thebpo-39925-pathlib-path-hard-link-arg-order branch 2 times, most recently fromec1bea4 toa65799eCompare

January 22, 2021 00:41

barneygale changed the title~~bpo-39950: addpathlib.Path.hardlink_to() method that supersedeslink_to()~~WIP: bpo-39950: addpathlib.Path.hardlink_to() method that supersedeslink_to()

Jan 22, 2021

Copy link

ContributorAuthor

barneygale commentedJan 22, 2021

Marking as WIP as I'll addressbpo-42999 first.

barneygale force-pushed thebpo-39925-pathlib-path-hard-link-arg-order branch 2 times, most recently fromde0b686 to2dfa391Compare

April 7, 2021 17:02

barneygale changed the title~~WIP: bpo-39950: addpathlib.Path.hardlink_to() method that supersedeslink_to()~~bpo-39950: addpathlib.Path.hardlink_to() method that supersedeslink_to()

Apr 7, 2021

Copy link

ContributorAuthor

barneygale commentedApr 7, 2021

Removed 'WIP' from the title. This is ready to review again! Thanks all.

Copy link

Member

brettcannon commentedApr 7, 2021

@barneygale please see#18909 (comment) on how to ask for a new round of reviews.

Copy link

ContributorAuthor

barneygale commentedApr 7, 2021

I have made the requested changes; please review again

bedevere-bot added awaiting change review and removed awaiting changes labels

Apr 7, 2021

Copy link

bedevere-bot commentedApr 7, 2021

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

bedevere-bot requested a review frombrettcannon

April 7, 2021 21:56

brettcannon approved these changes

Apr 9, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels

Apr 9, 2021

Copy link

Member

brettcannon commentedApr 9, 2021

LGTM!

@barneygale are you up for writing the "What's New" entry to document the addition of thehardlink_to() and the deprecation oflink_to()? It's okay if you're not, but it simplifies things for the release if it's updated now.

Copy link

ContributorAuthor

barneygale commentedApr 9, 2021

LGTM!
@barneygale are you up for writing the "What's New" entry to document the addition of thehardlink_to() and the deprecation oflink_to()? It's okay if you're not, but it simplifies things for the release if it's updated now.

I've added a new commit - hope it looks alright?

barneygale added2 commits

April 13, 2021 02:58

bpo-39925: addpathlib.Path.hardlink_to() method that supersedes `l…

e323969

…ink_to()`The argument order of `link_to()` is reversed, so:    a.link_to(b)Might be expected to create *a* as a link to *b*, in fact it creates *b*as a link to *a*. This doesn't match `symlink_to()` nor the documentationand doesn't seem to be the original author's intent.This commit deprecates `link_to()` and introduces `hardlink_to()`, whichhas the same argument order as `symlink_to()`.

Add info to "What's New" document

e506cae

barneygale force-pushed thebpo-39925-pathlib-path-hard-link-arg-order branch from94c5acb toe506caeCompare

April 13, 2021 02:00

barneygale requested a review frompitrou

April 13, 2021 10:44

Copy link

ContributorAuthor

barneygale commentedApr 19, 2021

@brettcannon sorry to bother, just bumping this review as I saw there's a feature freeze in two weeks! Thanks.

Copy link

Member

brettcannon commentedApr 21, 2021

@barneygale yep, this is the top of my review queue and I'm hoping to get to it this Friday (steering council stuff has to take priority, unfortunately).