Skip to content

refactor(webex): move configuration types into webex package - #5575

Merged
SoloJacobs merged 1 commit into
prometheus:mainfrom
TheMeier:chmaser-notifier-config-refactor-cbf65e1b
Sep 21, 2026
Merged

SoloJacobs merged 1 commit into
prometheus:mainfrom
TheMeier:chmaser-notifier-config-refactor-cbf65e1b

Conversation

@TheMeier

@TheMeier TheMeier commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Pull Request Checklist

Which user-facing changes does this PR introduce?

NONE

contains #5572 #5573 #5574

@TheMeier
TheMeier requested a review from a team as a code owner September 17, 2026 17:22
@TheMeier TheMeier changed the title chmaser notifier config refactor cbf65e1b refactor(webex): move configuration types into webex package Sep 17, 2026
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: prometheus/alertmanager/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fd8b2102-61a2-4025-8596-b611b6c3c797

📥 Commits

Reviewing files that changed from the base of the PR and between d8ce6f1 and 1de992d.

📒 Files selected for processing (4)
  • config/config.go
  • config/notifiers.go
  • config/notifiers_test.go
  • notify/webex/webex_test.go
💤 Files with no reviewable changes (2)
  • config/notifiers_test.go
  • config/notifiers.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Webex configuration now resides in notify/webex. The notifier, receiver configuration, and tests use the package-local WebexConfig type. The former configuration-package type, defaults, validation, and test were removed.

Changes

Webex configuration migration

Layer / File(s) Summary
Webex configuration contract
notify/webex/config.go, notify/webex/config_test.go, config/notifiers.go, config/notifiers_test.go
The WebexConfig type, default configuration, YAML unmarshalling, validation, and tests moved to notify/webex. The former declarations and test were removed from config.
Webex notifier type updates
notify/webex/webex.go, notify/webex/webex_test.go
The notifier constructor, stored configuration, and tests now use the local WebexConfig type.
Receiver configuration wiring
config/config.go
Receiver.WebexConfigs now uses webex.WebexConfig.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes sign-off, contribution compliance, release notes, and issue references, but it omits most required checklist items and does not clearly mark which items are not applicable. Complete the required checklist. Add the issue references under the issue section, mark applicable bugfix, feature, performance, and breaking-change items, confirm documentation status, and confirm that the related configuration tests cover…
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 18 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: moving Webex configuration types into the Webex package.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Resolution

Complete the required checklist. Add the issue references under the issue section, mark applicable bugfix, feature, performance, and breaking-change items, confirm documentation status, and confirm that the related configuration tests cover the exported API change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@TheMeier
TheMeier force-pushed the chmaser-notifier-config-refactor-cbf65e1b branch from 68e6e58 to d8ce6f1 Compare September 20, 2026 08:00

@SoloJacobs SoloJacobs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

small nit

expHeader string
}{
{
name: "with a valid message and a set http_config.authorization, it is formatted as expected",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think http_config.authorization is more correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Christoph Maser <christoph.maser+github@gmail.com>
@TheMeier
TheMeier force-pushed the chmaser-notifier-config-refactor-cbf65e1b branch from d8ce6f1 to 1de992d Compare September 21, 2026 08:39
@SoloJacobs
SoloJacobs enabled auto-merge (squash) September 21, 2026 08:49
@SoloJacobs
SoloJacobs merged commit 68e2c0a into prometheus:main Sep 21, 2026
7 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.

2 participants