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

Ensuring that the build script also cleans #17123

Merged
merged 3 commits into from
Aug 26, 2021

Conversation

sadasant
Copy link
Contributor

I thought rush build called prebuild! 😮

@sadasant sadasant requested a review from xirzec August 25, 2021 19:00
@sadasant sadasant self-assigned this Aug 25, 2021
@ghost ghost added the Azure.Identity label Aug 25, 2021
@sadasant sadasant force-pushed the identity/fix-clean branch from c804fa8 to d3a6e15 Compare August 25, 2021 19:01
Copy link
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

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

Maybe we should do this more broadly instead of just in identity?

@@ -31,7 +31,7 @@
"audit": "node ../../../common/scripts/rush-audit.js && rimraf node_modules package-lock.json && npm i --package-lock-only 2>&1 && npm audit",
"build:samples": "echo skipped",
"build:test": "tsc -p . && rollup -c 2>&1",
"build": "npm run extract-api && tsc -p . && rollup -c 2>&1",
"build": "npm run clean && npm run extract-api && tsc -p . && rollup -c 2>&1",
Copy link
Member

Choose a reason for hiding this comment

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

we should remove prebuild if we do this, since otherwise it'll duplicate when running the task directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok can do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did do the deed!

@sadasant sadasant changed the title [Identity] Ensuring that the build script also cleans Ensuring that the build script also cleans Aug 25, 2021
Copy link
Member

@maorleger maorleger left a comment

Choose a reason for hiding this comment

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

🚀

@ghost
Copy link

ghost commented Aug 26, 2021

Hello @sadasant!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@sadasant sadasant merged commit b8f46a9 into Azure:main Aug 26, 2021
@sadasant sadasant deleted the identity/fix-clean branch August 26, 2021 18:25
@xirzec
Copy link
Member

xirzec commented Aug 26, 2021

@sadasant thank you so much for having the energy to do this change. I didn't have it in me yesterday to attempt, but you have delivered. 👍

@sadasant
Copy link
Contributor Author

@xirzec that is too kind, Jeff. Thank you. I needed it.

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

Successfully merging this pull request may close these issues.

3 participants