Skip to content
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.

Sign up for GitHub

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

Jump to bottom

gh-88146: Set default netrc file name correctly on Windows #25732

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

gh-88146: Set default netrc file name correctly on Windows #25732

wants to merge 5 commits into from
+8 −1

Conversation

jheiselman
Copy link

@jheiselman jheiselman commented Apr 29, 2021 •
edited by bedevere-app bot
Loading

issue43980: Set default netrc file name correctly on Windows

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@jheiselman

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@jheiselman
Copy link
Author

I have now signed the CLA

eamanu
eamanu suggested changes May 26, 2021
View reviewed changes
Copy link
Contributor

@eamanu eamanu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@jheiselman Please rename title PR to bpo-NNNN: Set default.... [0]

Also add NEWS.

[0] https://devguide.python.org/

Lib/netrc.py Outdated
@@ -23,7 +23,10 @@ class netrc:
def __init__(self, file=None):
default_netrc = file is None
if file is None:
file = os.path.join(os.path.expanduser("~"), ".netrc")
default_file_name = ".netrc"
Copy link
Contributor

Choose a reason for hiding this comment

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

what about inline the if clause?

deafult_file_name = '.netrc' if os.name != 'nt' else '_netrc'

Copy link
Author

Choose a reason for hiding this comment

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

I’m good with that. I’m currently on vacation. Will be back in a week to update PR title and formatting.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 26, 2021
issue43980: Set default netrc file name correctly on Windows
fc37260 
This update to the commit address the conversation started by @eamanu
eamanu
eamanu approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

LGTM but you should follow the devguide [0], you must update
the issue title to bpo-43980: Set default netrc file name correctly on Windows

Also please add NEWS file.

[0] https://devguide.python.org/

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 7, 2022
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@netlify
Copy link

netlify bot commented Dec 7, 2022 •
edited
Loading

✅ Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit d5b6193
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/638fdb8a7af8c80008fbd323

@zooba zooba changed the title Set default netrc file name correctly on Windows bpo-43980: Set default netrc file name correctly on Windows Dec 7, 2022
@zooba
Copy link
Member

zooba commented Dec 7, 2022

Change itself looks fine, but definitely needs a NEWS entry and updated What's New

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Dec 7, 2022 •
edited
Loading

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

@jheiselman
Copy link
Author

Change itself looks fine, but definitely needs a NEWS entry and updated What's New

I will get to this tomorrow when I'm back at work. The bot explained how to do a NEWS entry, but @zooba mentioned an updated "What's New". Where are the instructions for providing that?

@zooba
Copy link
Member

zooba commented Jan 9, 2023

an updated "What's New". Where are the instructions for providing that?

Probably somewhere in the devguide, but basically we should have something listed in .\Doc\whatsnew\3.12.rst. There should be a section for "changed stdlib modules" which I think is appropriate.

@jheiselman jheiselman mannequin mentioned this pull request Jan 5, 2024
Open
@erlend-aasland erlend-aasland changed the title bpo-43980: Set default netrc file name correctly on Windows gh-88146: Set default netrc file name correctly on Windows Jan 5, 2024
@erlend-aasland
Copy link
Contributor

@jheiselman, can you retarget this for 3.13?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eamanu eamanu eamanu approved these changes

@zooba zooba Awaiting requested review from zooba

Assignees
No one assigned
Labels
awaiting core review OS-windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jheiselman @the-knights-who-say-ni @bedevere-bot @zooba @erlend-aasland @eamanu @iritkatriel @ezio-melotti