Skip to content

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