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

[Recorder] Coupling RecorderEnvironmentSetup with the record call #7083

Merged

Conversation

HarshaNalluru
Copy link
Member

@HarshaNalluru HarshaNalluru commented Jan 24, 2020

The changes in this PR are as per the discussions with @xirzec and @sadasant .

Description

Required for #5156

Copy link
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

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

I like this approach 👍

@HarshaNalluru HarshaNalluru marked this pull request as ready for review January 28, 2020 01:05
@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder labels Jan 28, 2020
@xirzec
Copy link
Member

xirzec commented Jan 28, 2020

Should we merge #7085 directly, since these changes aren't compatible with the existing tests?

@HarshaNalluru
Copy link
Member Author

Should we merge #7085 directly, since these changes aren't compatible with the existing tests?

Yes, enough to merge #7085, just waiting for some more approvals on it.

Copy link
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

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

Ship it!! :)

@HarshaNalluru HarshaNalluru merged commit ac112f1 into Azure:master Jan 28, 2020
HarshaNalluru added a commit that referenced this pull request Jan 28, 2020
…th the record call (#7085)

* add brand new RecorderEnvironmentSetup

* import RecorderEnvironmentSetup from the utils and pass on to setEnvironmentOnLoad

* update exports in the recorder

* import RecorderEnvironmentSetup in storage-queue

* update tests with recorderEnvSetup accordingly

* cleanup - remove dependency on env variables in replaceableVariables

* Update testutils.common.ts files for all storage packages

* file-share tests fixed

* blob tests fix

* blob fix leftovers

* recordings for the beforeEach update

* storage-file-datalake update recorderEnvSetup

* fix formatting

* DataLakeService FIX

* FILE DATALAKE - left overs

* updated Guidelines accordingly

* update keyvault tests

* Update Changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. test-utils-recorder Label for the issues related to the common recorder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants