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

[DOP-4097]: Ensure we use the --no-caching flag for production builds #940

Merged
merged 9 commits into from
Nov 30, 2023

Conversation

branberry
Copy link
Contributor

@branberry branberry commented Nov 21, 2023

Ticket

DOP-4097

Notes

To verify, I have added the flag for staging jobs to ensure that the flag is passed into the makefile without error. Here is a link to a job that has run with the --no-caching flag.

The shared.mk PR can be found here.

Copy link

The URL for your feature branch webhook is https://obrs6yxld6.execute-api.us-east-2.amazonaws.com/prod/

@branberry branberry changed the title [DOP-4097]: Add no caching flag for ts command [DOP-4097]: Ensure we use the --no-caching flag for production builds Nov 21, 2023
@branberry branberry requested a review from seungpark November 21, 2023 16:45
@branberry branberry marked this pull request as ready for review November 21, 2023 16:45
Copy link
Contributor

@seungpark seungpark left a comment

Choose a reason for hiding this comment

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

minor comments below! LGTM with the changes targeted for meta branch

@@ -18,5 +23,9 @@ export async function nextGenParse({ repoDir, patchId, commitHash }: NextGenPars
commandArgs.push(patchId);
}

if (isProd) {
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, this is for when we do use this export fn nextGenParse for prod, since this is not being used yet. can we add a comment for this?

@@ -60,6 +60,7 @@ export class StagingJobHandler extends JobHandler {

prepStageSpecificNextGenCommands(): void {
if (this.currJob.buildCommands) {
// TODO: Remove no-caching after done testing
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, can remove!

@branberry branberry merged commit 1c063bb into master Nov 30, 2023
7 checks passed
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.

4 participants