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

AddDataPartReferenceRelationshipInternal fix #954

Merged
merged 6 commits into from
Jul 20, 2021
Merged

AddDataPartReferenceRelationshipInternal fix #954

merged 6 commits into from
Jul 20, 2021

Conversation

tomjebo
Copy link
Collaborator

@tomjebo tomjebo commented Jun 24, 2021

fix AddDataPartReferenceRelationshipInternal call to system.io.packaging createrelationship to pass on id if available.

Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

Can you add a test so we don't regress this again in the future?

Co-authored-by: Taylor Southwick <tasou@microsoft.com>
@tomjebo tomjebo dismissed a stale review via 64528f9 July 20, 2021 20:18
@tomjebo tomjebo requested a review from twsouthwick July 20, 2021 20:56
Copy link
Member

@twsouthwick twsouthwick left a comment

Choose a reason for hiding this comment

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

LGTM. Can you update the CHANGELOG with the update


// Additional test for related 1issue #949
// Verify that the id requested in AddDataPartReferenceRelationship was used.
var mediaDataPart = new MediaDataPart(ppt.InternalOpenXmlPackage, MediaDataPartType.Mp3);
Copy link
Member

Choose a reason for hiding this comment

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

can you make this a separate test now that you're updating the file itself?

@tomjebo tomjebo merged commit 6937d89 into dotnet:main Jul 20, 2021
@tomjebo tomjebo deleted the issue949 branch June 15, 2022 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants