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

[storage][stg73] Changefeed follow-up #10827

Merged
merged 29 commits into from
Sep 4, 2020

Conversation

ljian3377
Copy link
Member

@ljian3377 ljian3377 commented Aug 25, 2020

Reference:
https://docs.microsoft.com/en-us/azure/storage/blobs/storage-blob-change-feed?tabs=azure-portal#understand-change-feed-organization
Azure/azure-sdk-for-net#13999
Azure/azure-sdk-for-net#13885
Azure/azure-sdk-for-net#14131
#9151

TODOs

Most change in storage-internal-avro are already reviewed in this PR: #10062, except for a bug fix in sdk/storage/storage-internal-avro/src/AvroReader.ts.

@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label Aug 25, 2020
@ljian3377 ljian3377 force-pushed the changefeed-followwup branch from 618bf9e to 317b9d8 Compare August 27, 2020 06:38
@jiacfan
Copy link
Member

jiacfan commented Sep 2, 2020

Please add Change log for change feed updates.

Copy link
Member

@jiacfan jiacfan left a comment

Choose a reason for hiding this comment

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

:shipit:

@ljian3377 ljian3377 added the APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. label Sep 3, 2020
@@ -0,0 +1,7 @@
# Release History

## 1.0.0 (Unreleased)
Copy link
Member

Choose a reason for hiding this comment

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

storage-internal-avro is not a published package? Since not, we don't need this changelog file.

Copy link
Member Author

@ljian3377 ljian3377 Sep 3, 2020

Choose a reason for hiding this comment

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

I'd like to keep the CHANGELOG.md as it is.
Since it's more like a package that's not published (added it to the rush.json as utility:

), I am basically following the routine of test-util-recorder: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/test-utils/recorder/CHANGELOG.md

@@ -0,0 +1,21 @@
The MIT License (MIT)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this license file, since changefeed package already has one in the released package.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above. Let's keep it as it is.

Copy link
Member

@bterlson bterlson left a comment

Choose a reason for hiding this comment

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

API changes seem good.

Copy link
Member

@XiaoningLiu XiaoningLiu left a comment

Choose a reason for hiding this comment

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

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIChange This PR contains an addition or change to the API signature and must be reviewed by an architect. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants