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

fix: add proxy support when fetching patches #121

Closed
wants to merge 3 commits into from
Closed

Conversation

timja
Copy link
Contributor

@timja timja commented Jan 5, 2018

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Adds proxy support when fetching patches, see #112

Let me know if you think this should be refactored into the lib/request/request file,

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

#112

What are the relevant tickets?

Screenshots

Additional questions

@odinn1984
Copy link
Contributor

@timja-kainos please run npm run lint and fix all of the linting errors and make sure tests are running correctly. Thanks!

@odinn1984
Copy link
Contributor

@timja-kainos please make sure that there is only one fix commit that contains the fix with the following format: "fix: what we fixed" also the branch name should be in the format of "fix/what_fixing"

also, this change needs to be documented as it may cause some un-expected behaviour for users that don't expect this.

@timja
Copy link
Contributor Author

timja commented Jan 8, 2018

@odinn1984 The docs say this should work https://snyk.io/docs/faqs/#using-snyk
I partially fixed this a couple of months ago after the migration from request to needle but I didn't notice that this part wasn't working and other priorities came up.
Are any additional docs changes needed?

@timja
Copy link
Contributor Author

timja commented Jan 8, 2018

Superseded by #122

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.

2 participants