Skip to content
This repository was archived by the owner on Oct 12, 2023. It is now read-only.

Fix subscription id removal#24

Merged
troydai merged 3 commits intomasterfrom
fix-subscription-id-removal
Jun 29, 2017
Merged

Fix subscription id removal#24
troydai merged 3 commits intomasterfrom
fix-subscription-id-removal

Conversation

@v-iam
Copy link
Contributor

@v-iam v-iam commented Jun 28, 2017

This PR fixes #16 and #23, to ensure that subscription ID is correctly removed in all cases that SubscriptionRecordingProcessor purports to address.

I recommend a new release (0.5.0, probably, since this changes the behavior) after this is merged. Failing to remove the subscription ID seems like a significant problem.

@msftclas
Copy link

@v-iam,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@troydai
Copy link
Contributor

troydai commented Jun 28, 2017

I'll test it first.

from 1.11.1 to 1.11.0 due to a sympton similar to this
kevin1024/vcrpy#318
@troydai
Copy link
Contributor

troydai commented Jun 29, 2017

Add a commit to this PR and it looks good to me. @v-iam you can merge and bump the version and release. I'll do some additional test on CLI side before we begin consuming new release.

@troydai troydai merged commit 2ddd9c7 into master Jun 29, 2017
@troydai troydai deleted the fix-subscription-id-removal branch June 29, 2017 19:15
@codecov-io
Copy link

codecov-io commented Jun 29, 2017

Codecov Report

Merging #24 into master will decrease coverage by 0.39%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #24     +/-   ##
=========================================
- Coverage   48.16%   47.76%   -0.4%     
=========================================
  Files          10       10             
  Lines         382      381      -1     
  Branches       58       61      +3     
=========================================
- Hits          184      182      -2     
  Misses        194      194             
- Partials        4        5      +1
Impacted Files Coverage Δ
...re_devtools/scenario_tests/recording_processors.py 55.44% <85.71%> (-1.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b9b34e...fdaf4d4. Read the comment docs.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subscription ID in response's location header is not correctly removed.

4 participants