Skip to content

Conversation

@lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Nov 6, 2018

Also added an example in shutil in order to make more clear how they are to be used.

Initially reported by Weinan Li on bpo.

https://bugs.python.org/issue22021

Automerge-Triggered-By: @gvanrossum

…ample in shutil in order to make more clear how they are to be used
archive; for example, we typically chdir into *root_dir* before creating the
archive.
archive. This also means that all paths in the archive will be relative to
*root_dir*.
Copy link

@remilapeyre remilapeyre Mar 4, 2019

Choose a reason for hiding this comment

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

What do you think of

*root_dir* is a directory that will be the root directory of the archive, all paths in the archive will be relative to it; for example, we typically chdir into *root_dir* before creating the archive. 

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like it better. I can remember having some difficulties in the wording of this. Thanks for your suggestion.

@remilapeyre
Copy link

Thanks for taking the time to improve the documentation, can you have a look as to why the CI is failing?

https://travis-ci.org/python/cpython/jobs/501463292

@lysnikolaou
Copy link
Member Author

@remilapeyre It turns out it was only a trailing whitespace, which is now fixed.

-rw-r--r-- tarek/staff 37192 2010-02-06 18:23:10 ./known_hosts
.. _shutil-archiving-example-with-basedir:
Copy link
Member

Choose a reason for hiding this comment

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

Could you please link to the example from the make_archive() documentation?

Copy link
Member Author

@lysnikolaou lysnikolaou May 13, 2019

Choose a reason for hiding this comment

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

Thanks @berkerpeksag for the review. From where exactly would you think it would be best to link? From the title itself or from somewhere in the text?

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking about adding something like "See :ref:`shutil-archiving-example-with-basedir` for how to use *base_dir* and *root_dir* together." after https://github.com/python/cpython/pull/10367/files#diff-bda6196613f849e0aa43c7e2dd9f6068R540

What do you think?

Copy link
Member Author

@lysnikolaou lysnikolaou May 13, 2019

Choose a reason for hiding this comment

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

That would be really nice actually. This way someone, who might not scroll all the way down, will not miss the example. I think it may be a better idea to not have a whole new paragraph for the "See the example" bit. It feels more natural to me to just append it at the end of the current paragraph. What is your view on that?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me!

@bedevere-bot
Copy link

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 phrase I 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.

…the example code, tree output for the directory structure
@lysnikolaou
Copy link
Member Author

@berkerpeksag, thanks for the review. I have made the requested changes; please review again. I have included a link to the make_archive example in the text, but I'm not really sure that's what you had in mind.

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@lysnikolaou
Copy link
Member Author

@berkerpeksag I have pushed the necessary changes, so the PR might be ready for another (final?) review.

@miss-islington miss-islington merged commit 7633371 into python:master Jun 8, 2020
@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

3 similar comments
@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

@miss-islington
Copy link
Contributor

Thanks @lysnikolaou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2020
…onGH-10367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
@bedevere-bot
Copy link

GH-20709 is a backport of this pull request to the 3.9 branch.

@bedevere-bot
Copy link

GH-20710 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2020
…onGH-10367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2020
…onGH-10367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
@bedevere-bot
Copy link

GH-20711 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-20712 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2020
…onGH-10367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
miss-islington added a commit that referenced this pull request Jun 8, 2020
…0367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
miss-islington added a commit that referenced this pull request Jun 8, 2020
…0367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
miss-islington added a commit that referenced this pull request Jun 8, 2020
…0367) Also added an example in shutil in order to make more clear how they are to be used. Initially reported by Weinan Li on bpo. (cherry picked from commit 7633371) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

8 participants