Skip to content

GH-4109: Add PublishReadyToRun to DotNetRestoreSettings#4110

Merged
devlead merged 1 commit intocake-build:developfrom
Marusyk:rmarusyk/4109
Jun 29, 2023
Merged

GH-4109: Add PublishReadyToRun to DotNetRestoreSettings#4110
devlead merged 1 commit intocake-build:developfrom
Marusyk:rmarusyk/4109

Conversation

@Marusyk
Copy link
Copy Markdown
Contributor

@Marusyk Marusyk commented Jan 21, 2023

Please review.

Fixes #4109

Copy link
Copy Markdown
Member

@augustoproiete augustoproiete left a comment

Choose a reason for hiding this comment

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

LGTM

@Marusyk
Copy link
Copy Markdown
Contributor Author

Marusyk commented Mar 31, 2023

so, can we merge it?

@augustoproiete augustoproiete changed the title Add PublishReadyToRun to DotNetRestoreSettings GH-4109: Add PublishReadyToRun to DotNetRestoreSettings Apr 2, 2023
@augustoproiete
Copy link
Copy Markdown
Member

Thanks for the ping @Marusyk . We're looking into the impact on addins, as this is a breaking change to the public API, to decide if we can merge it now for v3.1.0 or if it has to wait for the next major.

@Marusyk
Copy link
Copy Markdown
Contributor Author

Marusyk commented Apr 3, 2023

@augustoproiete could you explain why is this a breaking change?

@augustoproiete
Copy link
Copy Markdown
Member

@Marusyk It's a binary breaking change that would only affect existing Cake addins. Addins that were compiled with an existing Cake version are referencing a type called DotNetRestoreSettings that does not have the property PublishReadyToRun and thus technically not the same type.

i.e. CakevNext...DotNetRestoreSettings might not be convertible to Cakev3.0...DotNetRestoreSettings

@Marusyk
Copy link
Copy Markdown
Contributor Author

Marusyk commented Apr 4, 2023

but this is why we have versions, isn't it? I'm sure that Cake add-ins don't compile on the main branch and always use the latest source.
adding an optional parameter is not a breaking change from API perspective

@devlead devlead enabled auto-merge June 29, 2023 07:22
@devlead devlead merged commit 3861561 into cake-build:develop Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add PublishReadyToRun to DotNetRestoreSettings

3 participants