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

Fix casing issue in arm-synapse files #13493

Closed

Conversation

praveenkuttappan
Copy link
Member

Fix build break in management pr pipeline

@ghost ghost added the Synapse label Jan 30, 2021
@praveenkuttappan
Copy link
Member Author

/azp run js - mgmt - pr

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@praveenkuttappan praveenkuttappan marked this pull request as ready for review January 30, 2021 00:31
Copy link
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

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

Looks good to me, but please get somebody from the arm-synapse team to approve 🌞

@praveenkuttappan
Copy link
Member Author

@colawwj : Can you please have a look at this PR? This is to fix build break in mgmt - pr pipeline. Logs here:
https://dev.azure.com/azure-sdk/public/_build/results?buildId=710915&view=logs&j=ca395085-040a-526b-2ce8-bdc85f692774&t=2997d1dd-cd01-525b-c0a8-6ec662309496

@qiaozha
Copy link
Member

qiaozha commented Jan 30, 2021

Should we bump the package version ?

@praveenkuttappan
Copy link
Member Author

@qiaozha @colawwj I believe current version is not yet released. Do we still need to bump package version?
@ramya-rao-a : just fyi about this PR.

@ramya-rao-a
Copy link
Contributor

@praveenkuttappan,

The last change to this package was in #13434 which should have auto published the package. Are we saying that the auto publish failed due to the build break which in turn was due to the casing issue here?

@qiaozha,
Am not sure if the version bump check is made with what is already checked into the master branch or what has been released to npm. Since the last PR did not result in publishing we should be good with the current changes

Also, this is a purely auto generated package and should not ideally have manual changes. Do we know how the casing issue got introduced?

cc @colawwj who create the last PR for this package

@qiaozha
Copy link
Member

qiaozha commented Feb 1, 2021

This issue is caused by git insensitive of file name case change. The actually change should be made to fix this issue is:
sdk/synapse/arm-synapse/src/operations/workspaceManagedSqlServerRecoverableSqlpools.ts => sdk/synapse/arm-synapse/src/operations/workspaceManagedSqlServerRecoverableSqlPools.ts
and
sdk/synapse/arm-synapse/src/models/workspaceManagedSqlServerRecoverableSqlpoolsMappers.ts => sdk/synapse/arm-synapse/src/models/workspaceManagedSqlServerRecoverableSqlPoolsMappers.ts

Copy link
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

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

Can't merge this PR because of the actual changes needed which is based on latest swagger

@qiaozha
Copy link
Member

qiaozha commented Feb 1, 2021

@colawwj Can you submit another PR to fix this issue? Thanks

@praveenkuttappan
Copy link
Member Author

@colawwj @qiaozha : Can you please prioritize this fix? This is causing build break and impacting other PRs.

@qiaozha
Copy link
Member

qiaozha commented Feb 2, 2021

fixed by #13535 and the release pipeline is working again. a newer version of @azure/arm-synapse sdk is out here. https://www.npmjs.com/package/@azure/arm-synapse/v/5.0.0

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this pull request Mar 18, 2021
add go sdk config for apimanagement (Azure#13493)
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.

4 participants