Skip to content

Fix header misreporting to engine#1689

Merged
evans merged 2 commits into
masterfrom
private-headers
Sep 18, 2018
Merged

Fix header misreporting to engine#1689
evans merged 2 commits into
masterfrom
private-headers

Conversation

@JakeDawkins

@JakeDawkins JakeDawkins commented Sep 18, 2018

Copy link
Copy Markdown
Contributor

Issue

When using the privateHeaders option in the constructor, not all expected headers were being reported to Engine.

This was caused because the loop iterating over the headers had a break where it should have had a continue, causing the loop to be cut short.

Note: This bug didn't cause any private headers to get reported, so no worries there 😄

TODO:

  • Update CHANGELOG.md with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

@evans evans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@JakeDawkins Thank you for the PR! Not sure why we're getting package-lock differences. Are you running npm 6?

Also can you add this PR to the changelog?

@JakeDawkins

Copy link
Copy Markdown
Contributor Author

@evans done! I wasn't on npm 6 🙈 There are still a couple diffs, all changing http urls to https. Not sure what's going on there, but significantly fewer than before.

@evans evans merged commit f3df370 into master Sep 18, 2018
@evans evans deleted the private-headers branch September 18, 2018 20:29
@evans

evans commented Sep 18, 2018

Copy link
Copy Markdown
Contributor

@JakeDawkins Thank you 🌮

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants