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.

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

[chore] fix code owners #37291

Merged
merged 2 commits into from
Jan 17, 2025
Merged

[chore] fix code owners #37291

merged 2 commits into from
Jan 17, 2025

Conversation

songy23
Copy link
Member

@songy23 songy23 commented Jan 17, 2025

No description provided.

@songy23 songy23 added ci-cd CI, CD, testing, build issues Skip Changelog PRs that do not require a CHANGELOG.md entry labels Jan 17, 2025
@songy23 songy23 marked this pull request as ready for review January 17, 2025 16:39
@songy23 songy23 requested a review from a team as a code owner January 17, 2025 16:39
@songy23 songy23 requested a review from ChrsMark January 17, 2025 16:39
crobert-1
crobert-1 reviewed Jan 17, 2025
View reviewed changes
.github/CODEOWNERS
extension/observer/ecstaskobserver/ @open-telemetry/collector-contrib-approvers
extension/storage/filestorage/ @open-telemetry/collector-contrib-approvers
receiver/jmxreceiver/ @open-telemetry/collector-contrib-approvers
exporter/kineticaexporter/ @open-telemetry/collector-contrib-approvers
Copy link
Member

Choose a reason for hiding this comment

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

Why is extra whitespace being added here? I assume it's to fix CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, without the whitespace CI failed: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/12832998175/job/35787022502

(it's really hard to tell unless you select and highlight those diffs)

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with merging this now to fix CI, I realize it's time sensitive, but it seems like the real solution here is to fix githubgen to not add whitespace after these lines. I assume removing the trailing whitespace before \n should fix it?

opentelemetry-collector-contrib/cmd/githubgen/codeowners.go

Line 142 in 3febf16

unmaintainedCodeowners += fmt.Sprintf("%s/%s @open-telemetry/collector-contrib-approvers \n", key, strings.Repeat(" ", data.maxLength-len(key)))

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 created #37292 to follow up, will merge this PR to fix mainline for now

@songy23 songy23 mentioned this pull request Jan 17, 2025
Closed
@songy23 songy23 merged commit fa1e76b into open-telemetry:main Jan 17, 2025
167 checks passed
@songy23 songy23 deleted the fix-codeowners branch January 17, 2025 16:59
@github-actions github-actions bot added this to the next release milestone Jan 17, 2025
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Jan 26, 2025
@songy23 @chengchuanpeng
[chore] fix code owners (open-telemetry#37291)
5312ed4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crobert-1 crobert-1 crobert-1 left review comments

@evan-bradley evan-bradley evan-bradley approved these changes

@atoulme atoulme atoulme approved these changes

@ChrsMark ChrsMark Awaiting requested review from ChrsMark ChrsMark is a code owner automatically assigned from open-telemetry/collector-contrib-approvers

Assignees

@MovieStoreGuy MovieStoreGuy

Labels
ci-cd CI, CD, testing, build issues Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
v0.118.0
Development

Successfully merging this pull request may close these issues.
5 participants
@songy23 @atoulme @evan-bradley @crobert-1 @MovieStoreGuy