Skip to content

Conversation

hardlydearly
Copy link
Contributor

There is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

Copy link
Member

@bboreham bboreham 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 this, and welcome to the Prometheus project!

What you did is absolutely fine, but it made me think some code could now be simplified a bit further. Some ideas below.

Copy link
Member

@machine424 machine424 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!
let's have another pair of eyes look at this

@hardlydearly
Copy link
Contributor Author

lgtm, thanks! let's have another pair of eyes look at this

Thanks for your review.

Modified. Please review again.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors several code sections to replace manual loops with the standard library's slices.Contains, simplifying and clarifying membership checks.

  • Simplifies the code in postings, tests, labels, Kubernetes discovery, config, and main packages by replacing explicit loops with slices.Contains.
  • Improves overall readability without altering behavior.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tsdb/index/postings.go Replaced loop with slices.Contains to check for EmptyPostings().
rules/manager_test.go Simplified metric name check using slices.Contains.
model/labels/regexp.go Replaced explicit loops with slices.Contains in matchers.
model/labels/labels_common.go Used slices.Contains instead of loop for label filtering.
discovery/kubernetes/kubernetes.go Simplified role selector validation with slices.Contains.
config/config.go Reordered imports and replaced loop with slices.Contains in API version validation.
cmd/prometheus/main.go Refactored duplicate flag value check using slices.Contains.

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM, but you'll need to fix linting.

@hardlydearly
Copy link
Contributor Author

LGTM, but you'll need to fix linting.

Modified. Please review again.

Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Looking great! One small thing in imports.

Signed-off-by: hardlydearly <799511800@qq.com>
Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

LGTM

@machine424 machine424 merged commit ba4b058 into prometheus:main May 9, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants