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

[dev-tool] Removed repo checkout directory assumption #12681

Merged
merged 2 commits into from
Jan 21, 2021

Conversation

willmtemple
Copy link
Contributor

Closes #12287
("Windows" in the linked issue is a red herring, the same bug would apply to Linux and macOS)

dev-tool had a couple of places where it assumed the repository was cloned into a directory named "azure-sdk-for-js", of course that isn't necessarily true. This fixes that.

@willmtemple willmtemple added dev-tool Issues related to the Azure SDK for JS dev-tool EngSys This issue is impacting the engineering system. labels Nov 24, 2020
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.

LGTM, Please test it locally first if you did not already.


const printModes = ["info", "warn", "error", "success", "debug"] as const;

export type Fn<T = void> = (...values: any[]) => T;
export type ModeMap<T> = { [k in typeof printModes[number]]: T };

const DEV_TOOL_PATH = __dirname.split("dev-tool")[0];
Copy link
Member

Choose a reason for hiding this comment

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

Won't this fail if cloned path happened to have "dev-tool"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. __dirname.split(path.sep + "dev-tool" + path.sep).slice(0, -1).join(path.sep + "dev-tool" + path.sep) is probably more reliable as long as we take care not to have a folder named dev-tool anywhere under the dev-tool folder. The most robust solution is to recursively look for the package.json, but I was trying to avoid doing that in module initialization.

@kurtzeborn kurtzeborn added the Client This issue points to a problem in the data-plane of the library. label Dec 3, 2020
@ramya-rao-a ramya-rao-a merged commit d2c412e into Azure:master Jan 21, 2021
ljian3377 pushed a commit to ljian3377/azure-sdk-for-js that referenced this pull request Jan 22, 2021
* [dev-tool] Removed repo checkout directory assumption.

* Improved the logic even more.

Co-authored-by: Will Temple <[email protected]>
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this pull request Feb 10, 2021
[Hub Generated] Review request for Microsoft.Media to add version stable/2020-05-01 (Azure#12681)

* update to new swaggers

* fix some validation issues

* change ErrorResponse to types.json for test

* Add systemData to other entities

* Fix encoding json to not use errorResponse test

* fix validation error

* updating examples with latest fixes

* fixing a few validation errors

* fix validation errors

* Fix validation error.

* fix int max len issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. dev-tool Issues related to the Azure SDK for JS dev-tool EngSys This issue is impacting the engineering system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dev-tool] failingt test on Windows: 1) Project Resolution resolution finds dev-tool package
6 participants