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.

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

Forward options’ ssl.key even when non-enumerable #2394

Merged
merged 2 commits into from
Nov 9, 2020

Conversation

charmander
Copy link
Collaborator

@charmander charmander commented Oct 28, 2020

Fixes #2392.

@charmander charmander force-pushed the forward-non-enumerable-ssl-key branch 4 times, most recently from da2bae7 to 4c2fce2 Compare October 31, 2020 22:12
@brianc
Copy link
Owner

brianc commented Nov 4, 2020

Hey @charmander - thank you so much for diving in to this & trying to get it working. I know working with travis can be pretty incredibly painful...yesterday it took >2 hours for some tests to run. Would me upgrading to a paid plan help w/ the turn-around time there? Also, wondering if it'd be worthwhile for me to take the functional change to the code & write a unit test (and mock the ssl socket call) to check it's working and we can work on the larger travis integration tests in another PR? That way we can close the actual bug fast?

@charmander charmander force-pushed the forward-non-enumerable-ssl-key branch from 7587128 to 5f86903 Compare November 6, 2020 00:38
@charmander
Copy link
Collaborator Author

Would me upgrading to a paid plan help w/ the turn-around time there?

Hmm… well, how would you feel about a PR that switched the CI to GitHub Actions?

Also, wondering if it'd be worthwhile for me to take the functional change to the code & write a unit test (and mock the ssl socket call) to check it's working and we can work on the larger travis integration tests in another PR? That way we can close the actual bug fast?

I’ve decided to write just one integration test that runs in its own CI job. When the tests are refactored to read configuration consistently, it won’t be needed anymore :)

Sorry it took so long to get back to this.

@charmander charmander force-pushed the forward-non-enumerable-ssl-key branch from 8ed686f to d2cebd9 Compare November 6, 2020 01:18
@charmander charmander force-pushed the forward-non-enumerable-ssl-key branch from 3d35ed5 to b18e725 Compare November 6, 2020 07:34
@charmander charmander marked this pull request as ready for review November 6, 2020 08:56
@brianc
Copy link
Owner

brianc commented Nov 9, 2020

Hmm… well, how would you feel about a PR that switched the CI to GitHub Actions?

I would feel great about that.

I’ve decided to write just one integration test that runs in its own CI job. When the tests are refactored to read configuration consistently, it won’t be needed anymore :)

dope

Sorry it took so long to get back to this.

100% no worries at all. Take all the time you want! Sorry travis was blocking you.

@brianc brianc merged commit 0012a43 into brianc:master Nov 9, 2020
@brianc
Copy link
Owner

brianc commented Nov 10, 2020

release [email protected]

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.

Cert-based authentication broken
2 participants