Skip to content

Gallery 2022_03_03#29817

Merged
Yao725 merged 7 commits intoAzure:feature/gallery-2022-03-03from
tiregan:gallery-2022-03-03-tiregan
Sep 7, 2022
Merged

Gallery 2022_03_03#29817
Yao725 merged 7 commits intoAzure:feature/gallery-2022-03-03from
tiregan:gallery-2022-03-03-tiregan

Conversation

@tiregan
Copy link
Copy Markdown

@tiregan tiregan commented Jul 12, 2022

Contributing to the Azure SDK

Generated from swagger PR: Azure/azure-rest-api-specs#20398

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@check-enforcer
Copy link
Copy Markdown

This pull request is protected by Check Enforcer.
For more information about how to run a pipeline against this pull request, see this.

@Yao725
Copy link
Copy Markdown
Member

Yao725 commented Jul 15, 2022

/azp run net - mgmt - ci

@azure-pipelines
Copy link
Copy Markdown

Pull request contains merge conflicts.

Copy link
Copy Markdown
Member

@Yao725 Yao725 left a comment

Choose a reason for hiding this comment

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

@tiregan could you please resolve the merge conflicts?

/// shared to community.
/// </summary>
[JsonProperty(PropertyName = "communityGalleryInfo")]
public object CommunityGalleryInfo { get; set; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fixing a bug.

tiregan and others added 2 commits September 1, 2022 10:56
Copy link
Copy Markdown
Member

@Yao725 Yao725 left a comment

Choose a reason for hiding this comment

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

I still find some breaking changes in this PR. Could you please take a look at below comments to see if we can do something in customization to minimize the breaking change, thanks.

P.S: I think some of them might be hard to support the backward compatibility by customization. If so, we need to bump the major package version before release.

/// <param name="uri">The uri of the gallery artifact version source.
/// Currently used to specify vhd/blob source.</param>
public GalleryArtifactVersionSource(string id = default(string), string uri = default(string))
public GalleryArtifactVersionSource(string id = default(string))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a breaking change.

/// values are 'None', 'ReadOnly', and 'ReadWrite'. Possible values
/// include: 'None', 'ReadOnly', 'ReadWrite'</param>
public GalleryDataDiskImage(int lun, int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryArtifactVersionSource source = default(GalleryArtifactVersionSource))
public GalleryDataDiskImage(int lun, int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryDiskImageSource source = default(GalleryDiskImageSource))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Breaking change.

/// values are 'None', 'ReadOnly', and 'ReadWrite'. Possible values
/// include: 'None', 'ReadOnly', 'ReadWrite'</param>
public GalleryDiskImage(int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryArtifactVersionSource source = default(GalleryArtifactVersionSource))
public GalleryDiskImage(int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryDiskImageSource source = default(GalleryDiskImageSource))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Breaking change.

/// </summary>
/// <param name="dataDiskImages">A list of data disk images.</param>
public GalleryImageVersionStorageProfile(GalleryArtifactVersionSource source = default(GalleryArtifactVersionSource), GalleryOSDiskImage osDiskImage = default(GalleryOSDiskImage), IList<GalleryDataDiskImage> dataDiskImages = default(IList<GalleryDataDiskImage>))
public GalleryImageVersionStorageProfile(GalleryArtifactVersionFullSource source = default(GalleryArtifactVersionFullSource), GalleryOSDiskImage osDiskImage = default(GalleryOSDiskImage), IList<GalleryDataDiskImage> dataDiskImages = default(IList<GalleryDataDiskImage>))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Breaking change.

/// values are 'None', 'ReadOnly', and 'ReadWrite'. Possible values
/// include: 'None', 'ReadOnly', 'ReadWrite'</param>
public GalleryOSDiskImage(int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryArtifactVersionSource source = default(GalleryArtifactVersionSource))
public GalleryOSDiskImage(int? sizeInGB = default(int?), HostCaching? hostCaching = default(HostCaching?), GalleryDiskImageSource source = default(GalleryDiskImageSource))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Breaking change.

/// </summary>
[JsonProperty(PropertyName = "communityGalleryInfo")]
public object CommunityGalleryInfo { get; set; }
public CommunityGalleryInfo CommunityGalleryInfo { get; set; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's nice to change the type from object to the specific type CommunityGalleryInfo, but it is still a breaking change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Yao725 , and the breaking change was approved in the Swagger PR by the breaking change review board. Mostly because the functionality was broken without these changes.
I am unaware of any customizations that can be done to mitigate these different breaking changes. If you have any advice, please let us know.

@Yao725 Yao725 merged commit 4bf84f5 into Azure:feature/gallery-2022-03-03 Sep 7, 2022
@Sandido
Copy link
Copy Markdown
Contributor

Sandido commented Sep 9, 2022

I still find some breaking changes in this PR. Could you please take a look at below comments to see if we can do something in customization to minimize the breaking change, thanks.

P.S: I think some of them might be hard to support the backward compatibility by customization. If so, we need to bump the major package version before release.

Yes, the major version will be updated.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants