-
Notifications
You must be signed in to change notification settings - Fork 315
Add the --trusted option to upload and register commands #463
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
Conversation
Hi, sorry to fix all this so late, but these are done. |
I don't understand this failure. @sigmavirus24 Can you please explain the required fix ? |
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.
@glenfant From the link below, it looks like the failure is due to line length. I've made some suggestions to correct it.
https://travis-ci.org/pypa/twine/jobs/541750124#L235-L236
I got here by clicking the "Details" link in the "All checks have failed box" at the bottom of the PR, and then clicking on the "Linting code smells" failure.
FYI @glenfant you have merge conflicts here. |
Added the "--tusted" option (WIP) Added '--trusted' option on upload and register
Line length fixed Co-Authored-By: Brian Rutledge <bhrutledge@gmail.com>
Hi @bhrutledge all your comments are taken into account. Ready to merge ! |
@glenfant Thanks for your continued work on this. However, this PR is now showing a bunch of changes that have already been merged. I don't know how that happened, or how to fix it, but I'm wary of merging this as-is. If it were me, I'd probably close this PR, create a new branch off a fresh fetch of |
|
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.
Flagging this as needing changes before merging, namely git rebase
.
@glenfant Are you able to do the |
@di has some concerns about this feature: #387 (comment). |
It looks like it may have been me. I didn't intentionally merge, but it looks like in add0aea, I merged in unrelated commits. The solution would be to cherrypick the correct commits without that merge commit. I can do that. |
On further consideration, and looking at the actual commits in glenfant:master, I don't see that commit in his commit history... but what I do see is f54ea50, which merges two heads that seem to be doing the same thing. It looks like a rebase was attempted, then merged with itself... and the way github is detecting the ancestry, it's comparing against the wrong basis. Now I think the fix is to cherry-pick 8ee8bb2 onto 2da05c4 and then force-push that. |
I've fixed the repo history and resolved the conflicts with recent merges. Still, there's a pending concern about this change, so I'll wait for that to be resolved. |
I think I agree with the sentiments of di. I'd like for this feature to be an ugly hack and not a simple option. What if instead you had this script:
Call it |
Would be great to have it merged, thanks! |
I think the consensus on this issue is that the bypassing of the trusted mode can be readily handled by an intentionally-ugly workaround for environments that require it. |
any change of opinion? this would be a fantastic feature to have. |
No |
This is explained in issue #387 and helps people who prefer to ignore CA cert issues and skip server certificate verifications.
Any comment is welcome.
Fixes #387.