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

Added new metric application_buildpack #126

Merged

Conversation

adamspd
Copy link
Contributor

@adamspd adamspd commented May 22, 2024

No description provided.

Copy link

linux-foundation-easycla bot commented May 22, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@adamspd adamspd marked this pull request as draft May 23, 2024 06:12
@benjaminguttmann-avtq
Copy link
Contributor

@adamspd just to ask one upfront question, are all those commits related to the new application_buildpack metric? If not it would be appreciated if they could spread across different PRs which would make it way easier to review them

@adamspd adamspd force-pushed the PAASCFY-2654-ajouter-nouveaux-metrique branch from 7230117 to ab6ec61 Compare May 27, 2024 12:06
@adamspd
Copy link
Contributor Author

adamspd commented May 27, 2024

Hi @benjaminguttmann-avtq, the commits were not related to a single metric but to the addition of 2 new metrics and the modification of a metric to add more information. I separated them into 3 pull requests as requested and yes, you're right, it will be easier later if we need to go back on one of them.

@adamspd adamspd marked this pull request as ready for review May 27, 2024 12:10
Copy link
Member

@gmllt gmllt left a comment

Choose a reason for hiding this comment

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

the fetchAndFilterDroplets() should be added to the workers execution pool using c.worker.PushIf("droplets", c.fetchDroplets, filters.Droplets)

fetcher/fetcher.go Outdated Show resolved Hide resolved
@adamspd adamspd requested a review from gmllt May 30, 2024 09:39
@adamspd adamspd marked this pull request as draft June 4, 2024 09:28
@adamspd adamspd marked this pull request as ready for review June 10, 2024 08:25
collectors/applications.go Outdated Show resolved Hide resolved
@adamspd adamspd requested a review from gmllt June 12, 2024 13:57
@benjaminguttmann-avtq
Copy link
Contributor

@adamspd Please make sure to run go test -v ./... as well as golangci-lint run to see if there are any issues

@adamspd
Copy link
Contributor Author

adamspd commented Jun 14, 2024

@adamspd Please make sure to run go test -v ./... as well as golangci-lint run to see if there are any issues

Absolutely.

@gmllt gmllt force-pushed the PAASCFY-2654-ajouter-nouveaux-metrique branch from 42dd55d to ce93f84 Compare June 18, 2024 07:28
fetcher/fetcher.go Outdated Show resolved Hide resolved
Copy link
Member

@gmllt gmllt 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 @adamspd

@gmllt gmllt merged commit 686e22b into cloudfoundry:master Jun 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants