Skip to content

Conversation

@vors
Copy link
Collaborator

@vors vors commented Feb 24, 2018

PR Summary

Follow-up to rushedly merged #6235 to restore the original value of the SecurityProtocol.

PR Checklist

@markekraus
Copy link
Contributor

Hmmm if we went this way, we would need to wrap all of the calls to github and not just this one.
I think It is fairly safe to use my PR (#6236) instead. That just enables TLS 1.2 in addition to the current settings and then it will stick for at least the current session.

It's not clean, but we just have too many github calls for various build tools and actions. So we can safely treat enabling TLS 1.2 as a bootstrap requirement.

Copy link
Contributor

@markekraus markekraus left a comment

Choose a reason for hiding this comment

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

I'd rather accept my change see comment

@vors
Copy link
Collaborator Author

vors commented Feb 24, 2018

I see pros and cons in both approaches.
I merged @markekraus PR. I think there is still value in doing it idempotently, but it's really negligible, so I will abandon this PR.

@vors vors closed this Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants