Skip to content

Add cacheTo argument to ci action #300

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
Merged
samruddhikhandale merged 5 commits into from
Dec 19, 2024
Merged

Add cacheTo argument to ci action #300

samruddhikhandale merged 5 commits into from
Dec 19, 2024

Conversation

@sebst
Copy link
Contributor

@sebst sebst commented Sep 2, 2024

Added the argument cacheTo to the action (#299) so that a cache image can be written to a registry.

@sebst sebst requested review from a team and stuartleeks as code owners September 2, 2024 15:29
@sebst
Copy link
Contributor Author

sebst commented Sep 2, 2024

@microsoft-github-policy-service agree

chrmarti
chrmarti requested changes Sep 3, 2024
View reviewed changes
Copy link
Collaborator

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Left a few comments.

@sebst
Copy link
Contributor Author

sebst commented Sep 3, 2024

Thanks for the PR! Left a few comments.

Thanks for having a look. I'll address those comments!

@sebst sebst requested a review from chrmarti September 3, 2024 13:14
@sebst
Copy link
Contributor Author

sebst commented Sep 18, 2024

Any update?

@sebst sebst mentioned this pull request Oct 8, 2024
Closed
samruddhikhandale
samruddhikhandale requested changes Oct 8, 2024
View reviewed changes
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Thanks, overall it looks good.

Left one comment

@sebst @samruddhikhandale
Update github-action/src/main.ts
c757f7c 
Typo

Co-authored-by: Samruddhi Khandale <[email protected]>
@sebst
Copy link
Contributor Author

sebst commented Oct 8, 2024

Thanks, overall it looks good.

Left one comment

Thanks! Fixed the typo

@samruddhikhandale
Copy link
Member

samruddhikhandale commented Oct 8, 2024

Ensure you have run scripts/build-local.sh

@sebst Can you help fix the failing action, see here

@sebst
Copy link
Contributor Author

sebst commented Oct 9, 2024

Ensure you have run scripts/build-local.sh

@sebst Can you help fix the failing action, see here

Done

@sebst
Copy link
Contributor Author

sebst commented Oct 10, 2024

@samruddhikhandale could you please restart the pipeline to see if the last commit fixes the issue?

@sebst
Copy link
Contributor Author

sebst commented Oct 14, 2024

Can this be merged now?

@samruddhikhandale @chrmarti @stuartleeks @bamurtaugh

@samruddhikhandale samruddhikhandale requested review from chrmarti and removed request for chrmarti and stuartleeks October 14, 2024 17:43
@samruddhikhandale
Copy link
Member

samruddhikhandale commented Oct 14, 2024

Hi @sebst, thanks for your patience.
We are waiting for another review from @chrmarti, we'd merge shortly. Thanks!

@sebst
Copy link
Contributor Author

sebst commented Oct 22, 2024

Just a friendly reminder.

@samruddhikhandale @chrmarti @stuartleeks @bamurtaugh

@sebst
Copy link
Contributor Author

sebst commented Nov 4, 2024

Just another friendly reminder.

@samruddhikhandale @chrmarti @stuartleeks @bamurtaugh

@sebst
Copy link
Contributor Author

sebst commented Nov 9, 2024

Hi @sebst, thanks for your patience.

We are waiting for another review from @chrmarti, we'd merge shortly. Thanks!

Is there any chance we get that merged?

@chrmarti

@stuartleeks
Copy link
Collaborator

stuartleeks commented Nov 29, 2024

@chrmarti - it looks like the PR feedback has been addressed. Do you have time to re-review?

@sebst
Copy link
Contributor Author

sebst commented Dec 10, 2024

@chrmarti - it looks like the PR feedback has been addressed. Do you have time to re-review?

Would be nice!

@sebst
Copy link
Contributor Author

sebst commented Dec 12, 2024

@samruddhikhandale @chrmarti @stuartleeks @bamurtaugh

This PR is open for more than 3 months now but it's just a small change.

Is there anything I could do on my end to get this merged?

Appreciate any feedback!

@sebst sebst mentioned this pull request Dec 12, 2024
Open
chrmarti
chrmarti approved these changes Dec 19, 2024
View reviewed changes
Copy link
Collaborator

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@samruddhikhandale samruddhikhandale merged commit 3998f9e into devcontainers:main Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrmarti chrmarti chrmarti approved these changes

@samruddhikhandale samruddhikhandale samruddhikhandale approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sebst @samruddhikhandale @stuartleeks @chrmarti