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.

Sign up for GitHub

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

Jump to bottom

[engsys] Remove typedoc dependency #22237

Merged
merged 3 commits into from
Jun 22, 2022
Merged

[engsys] Remove typedoc dependency #22237

merged 3 commits into from
Jun 22, 2022

Conversation

timovv
Copy link
Member

@timovv timovv commented Jun 14, 2022 •
edited
Loading

Packages impacted by this PR

  • @azure/arm-appservice-rest

Issues associated with this PR

Describe the problem that is addressed by this PR

Removes unnecessary dependency typedoc

@ghost ghost added the Mgmt This issue is related to a management-plane library. label Jun 14, 2022
deyaaeldeen
deyaaeldeen reviewed Jun 14, 2022
View reviewed changes
sdk/appservice/arm-appservice-rest/package.json Outdated
@@ -89,7 +89,7 @@
"prettier": "2.2.1",
"rimraf": "^3.0.0",
"source-map-support": "^0.5.9",
"typedoc": "0.15.2",
"typedoc": "^0.22.17",
Copy link
Member

Choose a reason for hiding this comment

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

@joheredi why does this package use typedoc?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a valid question, and I was sort of wondering myself. It's being used in the docs npm script and nowhere else. Running that script generates docs, of course, but I have no idea why this particular package needs that.

Copy link
Member

Choose a reason for hiding this comment

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

Individual package should stop having "typedoc" as their dev deps. This one might be from code-gen a while ago. There was also discussion on whether we should upgrade for 0.x.x versions. PNPM consider them as available upgrades. However, 0.x.x versions are not as stable as non-zero major versions.

Copy link
Member

Choose a reason for hiding this comment

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

Individual package should stop having "typedoc" as their dev deps. This one might be from code-gen a while ago. There was also discussion on whether we should upgrade for 0.x.x versions. PNPM consider them as available upgrades. However, 0.x.x versions are not as stable as non-zero major versions.

SemVer has special handling for this, ^0.1.2 will be compatible with 0.1.3 but not 0.2.0. It looks for the first non zero number being incremented.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, in that case, I am thinking we can drop this dependency altogether -- if that NPM script isn't getting used then it shouldn't be a problem.

Copy link
Member

Choose a reason for hiding this comment

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

BTW this is an "experimental" management plane Rest Client

Copy link
Member

Choose a reason for hiding this comment

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

Sent out this PR
Azure/autorest.typescript#1419

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @joheredi, should I close this PR then and wait for things to come through from the codegen side?

Copy link
Member

Choose a reason for hiding this comment

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

I have updated the codegen, in the meantime would you mind removing the dependency and script manually in this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@timovv timovv changed the title [engsys] Update dependency typedoc to ^0.22.17 [engsys] Remove typedoc dependency Jun 22, 2022
@timovv timovv requested a review from joheredi June 22, 2022 17:21
deyaaeldeen
deyaaeldeen approved these changes Jun 22, 2022
View reviewed changes
Copy link
Member

@deyaaeldeen deyaaeldeen left a comment

Choose a reason for hiding this comment

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

🚀

@timovv timovv merged commit 03190fd into Azure:main Jun 22, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this pull request Feb 1, 2023
@komo23x @namanag16
@ZhidaLiu @libc16
CodeGen from PR 21761 in Azure/azure-rest-api-specs
0029830 
Machinelearningservices microsoft.machine learning services 2022 12 01 preview (Azure#21761)

* Adds base for updating Microsoft.MachineLearningServices from version preview/2022-10-01-preview to version 2022-12-01-preview

* Updates readme

* Updates API version in new specs and examples

* Add Dec API Registries Swagger (Azure#21419)

* add december registries swagger + examples

* add status code 202 in examples

* fix 202 examples

* fixes

* fixes

* fix

* add 202 back in for put/patch

Co-authored-by: Komal Yadav <komalyadav@microsoft.com>

* remove location (Azure#21430)

Co-authored-by: Komal Yadav <komalyadav@microsoft.com>

* remove readonly flag on schedules property for CI (Azure#21653)

Co-authored-by: Naman Agarwal <naagarw@microsoft.com>

* add missing workspace properties (Azure#21725)

* December preview updating mfe.json specs (Azure#21510)

* December preview updating mfe.json specs

* MFE Dec 2022 Preview API - Adding logbase

* MFE 2022-12-01-preview swagger spec model validation fix

* MFE 2022-12-01-preview swagger spec model validation fix, add missing location

* MFE 2022-12-01-preview swagger spec model validation - typo fix

* MFE 2022-12-01-preview swagger spec model validation - fix api version in automljob example

* MFE 2022-12-01-preview swagger spec model validation - fix for multiselectenabled error

* MFE 2022-12-01-preview swagger spec model validation - fix for multiselectenabled error

* Fix  for 1006 - RemovedDefinition (RecurrenceTrigger,CronTrigger) (Azure#21822)

* fix ReadonlyPropertyChanged of MLC (Azure#21814)

Co-authored-by: Bingchen Li <bingchenli@microsoft.com>

* fixed custom-words conflict (Azure#21829)

* fix custom-words conflict merge (Azure#21830)

* example fix (INVALID_REQUEST_PARAMETER) (Azure#21832)

Co-authored-by: Ivaliy Ivanov <ivaliyivanov@Ivaliys-MacBook-Air.local>

* example fix, use correct api preview version  - (INVALID_REQUEST_PARAMETER) (Azure#21833)

Co-authored-by: Ivaliy Ivanov <ivaliyivanov@Ivaliys-MacBook-Air.local>

* Revert breaking change for MLC swagger 2022-12-01-preview (Azure#21885)

Co-authored-by: Bingchen Li <bingchenli@microsoft.com>

* Revert Connection Category back to enum. (Azure#21939)

* revert provisioning state change (Azure#21940)

* remove body (Azure#21978)

Co-authored-by: Komal Yadav <komalyadav@microsoft.com>

* Addressed comments, added x-ms-long-running-operation to a patch call (Azure#22005)

* Addressed comments, added x-ms-long-running-operation to a patch call

* fix examples for patch - remove body

* fixed formatting

* Ivalbert fix patch2 (Azure#22006)

* Addressed comments, added x-ms-long-running-operation to a patch call

* fix examples for patch - remove body

* fixed formatting

* fixed formatting

* Updated custom words (Azure#22262)

* Fixed prettier errors (Azure#22237)

* fixed examples for LRO_RESPONSE_HEADER check (Azure#22293)

* fixed examples for LRO_RESPONSE_HEADER check (Azure#22294)

* Example fix - OBJECT_MISSING_REQUIRED_PROPERTY - Missing required property: triggerType (#22317)

---------

Co-authored-by: Komal Yadav <23komal.yadav23@gmail.com>
Co-authored-by: Komal Yadav <komalyadav@microsoft.com>
Co-authored-by: Naman Agarwal <namanag16@gmail.com>
Co-authored-by: Naman Agarwal <naagarw@microsoft.com>
Co-authored-by: ZhidaLiu <zhili@microsoft.com>
Co-authored-by: libc16 <88697960+libc16@users.noreply.github.com>
Co-authored-by: Bingchen Li <bingchenli@microsoft.com>
Co-authored-by: Ivaliy Ivanov <ivaliyivanov@Ivaliys-MacBook-Air.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deyaaeldeen deyaaeldeen deyaaeldeen approved these changes

@praveenkuttappan praveenkuttappan Awaiting requested review from praveenkuttappan

@witemple-msft witemple-msft Awaiting requested review from witemple-msft

@jeremymeng jeremymeng Awaiting requested review from jeremymeng

@xirzec xirzec Awaiting requested review from xirzec

@joheredi joheredi Awaiting requested review from joheredi

Assignees
No one assigned
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dependency package typedoc has a new version available
6 participants
@timovv @azure-sdk @xirzec @deyaaeldeen @jeremymeng @joheredi