-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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] Followup of #7083 - Coupling RecorderEnvironmentSetup
with the record call
#7085
[Recorder] Followup of #7083 - Coupling RecorderEnvironmentSetup
with the record call
#7085
Conversation
Are we adding CHANGELOG entries to the recorder? I wonder if we should start. Other than that, once this PR works, I think we'll be good to go. This is probably the happiest path at the moment. Also, any thoughts on unit tests for this? |
Good suggestion. I was thinking too. Still, a changelog with just the dates would be good for keeping track things. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usage looks good too!
…6-soft-record-prereq
…followup/7083-storage-queue
…followup/7083-storage-queue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
], | ||
queryParametersToSkip: [] | ||
}; | ||
const recorder = record(that, recorderEnvSetup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this!!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you!
Tested the playback locally for all the storage and keyvault packages at 0fb53c7. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
…6-soft-record-prereq
…followup/7083-storage-queue
The changes in this PR are as per the discussions with @xirzec and @sadasant .
Changes in the PR
setReplaceableVariables
,setReplacements
,skipQueryParams
methods of the recorder are no longer exposed.RecorderEnvironmentSetup
type is being introduced. An object of this type is expected to be passed in therecord()
call.record(this)
->record(this, recorderEnvSetup)
.RecorderEnvironmentSetup
with the record call #7083 to make the build succeedRecorderEnvironmentSetup
with the record call #7083 has the changes corresponding to the recorder package only).