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

Ignore certificate errors #85

Merged
merged 3 commits into from
Aug 31, 2018
Merged

Ignore certificate errors #85

merged 3 commits into from
Aug 31, 2018

Conversation

debugwand
Copy link
Contributor

@simonplend
Copy link
Contributor

simonplend commented Aug 23, 2018 •
edited
Loading

I don't think it's desirable to always ignore certificate errors as it could lead to us missing real problems, but I agree that it's needed in some contexts. Could the ignoring of certificates be configurable as a CLI flag to n-test?

@adgad
Copy link
Collaborator

adgad commented Aug 23, 2018 •
edited
Loading

I think for most contexts we run in it's fine? I guess it's only really www.ft.com we'd care about...maybe only doing for NODE_ENV=development Actually that feels too magic.

@debugwand
Copy link
Contributor Author

I've taken both onboard and 1) added an environment check and 2) put in instructions for how to do that in the readme. It seems like not everyone has problems so the feature should be off by default and its down to the local dev to handle it.

@debugwand debugwand merged commit 9d43e1b into master Aug 31, 2018
@debugwand debugwand deleted the smoke-ignore-cert-errors branch August 31, 2018 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adgad adgad adgad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@debugwand @simonplend @adgad