Skip to content

Handle packages in Microsoft scopeto support renaming of opentelemetry#12458

Merged
praveenkuttappan merged 7 commits intoAzure:masterfrom
praveenkuttappan:rename_monitor
Nov 17, 2020
Merged

Handle packages in Microsoft scopeto support renaming of opentelemetry#12458
praveenkuttappan merged 7 commits intoAzure:masterfrom
praveenkuttappan:rename_monitor

Conversation

@praveenkuttappan
Copy link
Copy Markdown
Member

Support packages in both azure and microsoft scope and allow packages with "azure-" inside pacakge name. for e.g. openetelemetry-azure-monitor"

@weshaggard
Copy link
Copy Markdown
Member

cc @sima-zhu

@praveenkuttappan there are some other places like https://github.com/Azure/azure-sdk-for-js/blob/master/eng/tools/generate-static-index/templates/matthews/styles/main.js#L109 which is used by the github.io docs site.

I also expect this is going to require some changes in other places like our package index https://github.com/Azure/azure-sdk/blob/master/_includes/releases/variables/js.md we will likely need to figure out how to handle this in those other places.

@weshaggard
Copy link
Copy Markdown
Member

https://github.com/Azure/azure-sdk/blob/master/eng/scripts/Update-Release-Versions.ps1#L42 another place. I will help fix the issues in the azure-sdk repo once things get through the process and reach there.

@praveenkuttappan
Copy link
Copy Markdown
Member Author

cc @sima-zhu

@praveenkuttappan there are some other places like https://github.com/Azure/azure-sdk-for-js/blob/master/eng/tools/generate-static-index/templates/matthews/styles/main.js#L109 which is used by the github.io docs site.

I also expect this is going to require some changes in other places like our package index https://github.com/Azure/azure-sdk/blob/master/_includes/releases/variables/js.md we will likely need to figure out how to handle this in those other places.

I have included change in eng/tools/generate-static-index/templates/matthews/styles/main.js. I will check other places and submit another PR after unblocking renaming PR itself. I assume we may need more manual steps to handle renaming in doc side too.

@praveenkuttappan
Copy link
Copy Markdown
Member Author

@praveenkuttappan
Copy link
Copy Markdown
Member Author

I will need additional changed in dependency testing. But I will submit it as a separate PR once opentelemetry changes are unblocked

if ($dirList.Length -eq 1)
{
$DocVersion = $dirList[0].Name
$pkgs, $parsePkgInfoFn = RetrievePackages "NPM" $PublicArtifactLocation
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.

Why not call Get-javascript-PackageInfoFromPackageFile directly?

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.

Also the repository arguement is being retired from RetrievePackages function. So you should not need to pass NPN to the function.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why not call Get-javascript-PackageInfoFromPackageFile directly?

I was trying to reuse Retrievepackages function to get package details from artifact path. But I makes sense to just call Get-ChildItem in Language-Settings itself to decouple from common method.

@xirzec
Copy link
Copy Markdown
Member

xirzec commented Nov 16, 2020

This PR is still blocking the beta release of monitor-exporter - any chance we can get this buttoned up soon? We have an arch board review on Wednesday.

@praveenkuttappan
Copy link
Copy Markdown
Member Author

This PR is still blocking the beta release of monitor-exporter - any chance we can get this buttoned up soon? We have an arch board review on Wednesday.

I am making few more changes as per review comment and we should be ready soon.

@praveenkuttappan
Copy link
Copy Markdown
Member Author

I have tested a template release with this change. https://dev.azure.com/azure-sdk/internal/_build/results?buildId=621647&view=logs&j=ef505d45-100b-5890-ec9f-7c3786c71051&t=fb29220d-c659-58cb-9963-e1e196409dcc

@weshaggard : Can you please review and approve if current change looks fine?

Copy link
Copy Markdown
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

A couple questions but I think the changes look good.

@praveenkuttappan praveenkuttappan merged commit 0b075e4 into Azure:master Nov 17, 2020
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.

5 participants