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

feat: Add metrics-proxy rock #267

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

IbraAoad
Copy link
Contributor

@IbraAoad IbraAoad commented Oct 17, 2024

Ping the @canonical/rocks team.


Description

This PR adds the metrics-proxy ROCK to the OCI factory.

Related issues

#268


Picture of a cool rock:
cool-rock

@IbraAoad IbraAoad marked this pull request as ready for review October 17, 2024 13:16
@IbraAoad IbraAoad requested a review from a team as a code owner October 17, 2024 13:16
@IbraAoad IbraAoad changed the title [WIP] feat: Add metrics-proxy rock feat: Add metrics-proxy rock Oct 17, 2024
@IbraAoad
Copy link
Contributor Author

IbraAoad commented Dec 3, 2024

@cjdcordeiro Do you mind taking a look at this as well, thanks! 🙇‍♂️

Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

It lgtm. Thanks. Since it's the 1st PR, I'd also like another colleague to have a 2nd look before merging

FYI @clay-lake @zhijie-yang

Copy link
Collaborator

@zhijie-yang zhijie-yang left a comment

Choose a reason for hiding this comment

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

Looks nice to me, thanks! A nitpick for future updates: Please leave a blank line at the end of each file.

Copy link
Contributor

@clay-lake clay-lake left a comment

Choose a reason for hiding this comment

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

Would it be possible to add a newline at every EOF? I also added a comment about the prior version.

upload:
- source: canonical/metrics-proxy-rock
commit: 66763a5703df2e3175b87c0da69b29103da47169
directory: "0.1.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding the 0.1.0 rock as well?

@clay-lake
Copy link
Contributor

Quick note: we are waiting for #308 to be merged in. Should be completed shortly.

Copy link
Contributor

@clay-lake clay-lake left a comment

Choose a reason for hiding this comment

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

#308 is merged. We should be good to go!

@clay-lake clay-lake merged commit faeab52 into canonical:main Dec 11, 2024
31 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