Skip to content

moved arm-appservice#2130

Merged
KarishmaGhiya merged 1 commit intoAzure:masterfrom
KarishmaGhiya:moveArmAppAervice
Apr 10, 2019
Merged

moved arm-appservice#2130
KarishmaGhiya merged 1 commit intoAzure:masterfrom
KarishmaGhiya:moveArmAppAervice

Conversation

@KarishmaGhiya
Copy link
Member

@KarishmaGhiya KarishmaGhiya commented Apr 10, 2019

Moving packages for arm-appservice from packages@Azure and putting them under an sdk directory.
A few changes that have been made:

The source code directory lib has been renamed to src
All the links in various files referencing lib have been updated to src
The paths of all the data-plane package under appservice service (@azure/arm-appservice) have been updated eg: packages/@azure/arm-appservice to the new structure path sdk/appservice/arm-appservice to follow the structure sdk/servicename/packagename
All the corresponding links in different files like package.json, etc have also been updated for the new structure path
I have also updated the path in README.md (typescript) for azure-rest-api-specs.

Please review Azure/azure-rest-api-specs#5608
@salameer @daschult @kpajdzik @Azure/azure-sdk-eng

Copy link
Member

Choose a reason for hiding this comment

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

@kpajdzik do you know why each management package has its own copy of the license file? There shouldn't be any reason we cannot share one for the entire repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think NPM is going to display correct license if it isn't shipped with the package.

Copy link
Member

Choose a reason for hiding this comment

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

I don't even know if the file is necessary for the standard licenses like MIT as the package.json simply has "license": "MIT", but the file does automatically get included in the package according to the spec https://docs.npmjs.com/files/package.json#files so if we did move it we would have to add an entry for it in our package.json manaully. For now lets leave it but it is something we should potentially clean-up at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, you may be right.

Copy link
Member

Choose a reason for hiding this comment

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

I filed issue #2150 to follow-up on this conversation.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants