-
Notifications
You must be signed in to change notification settings - Fork 273
Move builds to pipelines.ts and update tests #478
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
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Consider only changing the remaining build names, to avoid misconception there is a domain build
. Given we changed to domain pipeline
, we keep: pipelines_get_build_log
other than build_get_log
(we are on public preview, so fine aligning those)
|
||
constructor(domainsInput?: string | string[]) { | ||
this.enabledDomains = new Set(); | ||
const normalizedInput = DomainsManager.parseDomainsInput(domainsInput); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are not using normalized name anymore we can remove the method parseDomainsInput
from class normalizedInput
just please.
Co-authored-by: Marcelo Novaes <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Thank you, looks great! 🥇
Moved build and pipeline tools to pipelines.ts
updates tests
renamed tools with prefixes for better naming
removed releases.ts and tools per PM request
GitHub issue number
#477
Associated Risks
Larger change, but low risk
✅ PR Checklist
🧪 How did you test it?
Manually
Added domain to test that
updated tests and re-rean them all