Skip to content

Address review comments#599

Merged
dbasunag merged 1 commit intoopendatahub-io:mainfrom
dbasunag:mc_address_comments
Sep 9, 2025
Merged

Address review comments#599
dbasunag merged 1 commit intoopendatahub-io:mainfrom
dbasunag:mc_address_comments

Conversation

@dbasunag
Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag commented Sep 9, 2025

Description

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • Tests
    • Reduced noisy debug/info logs in the test suite to streamline output and improve readability during local and CI runs.
    • Lowers log volume, making failures easier to spot and speeding up log navigation.
    • No changes to product behavior or APIs; execution and results remain unchanged, with only test verbosity adjusted.

@dbasunag dbasunag requested review from fege and lugi0 as code owners September 9, 2025 15:26
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 9, 2025

📝 Walkthrough

Walkthrough

Removed debug/info logging statements from tests in tests/model_registry/model_catalog/test_custom_model_catalog.py across several test cases. No test logic, API calls, or public interfaces were changed.

Changes

Cohort / File(s) Summary
Tests logging cleanup
tests/model_registry/model_catalog/test_custom_model_catalog.py
Deleted multiple debug/info log statements in tests: get_models_by_source, get_model_by_name, get_model_artifact, add_model, remove_model. No functional changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks (1 passed, 2 inconclusive)

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The current title "Address review comments" is too generic and fails to specify the main change of removing debug and info logging statements from model catalog tests. It does not provide enough context for reviewers or future maintainers. A more descriptive title would improve clarity in the project history. Update the title to clearly reflect the primary change, for example "Remove debug logging statements from model catalog tests" to concisely convey the purpose. Ensure the title remains concise and focused on the main update. This will help teammates understand the change at a glance.
Description Check ❓ Inconclusive The PR description currently includes only template placeholders with no actual summary, details of logging removal, or testing instructions. It provides no meaningful information about what was changed or how it was verified. A complete description is needed for reviewer understanding and decision-making. Populate the description with a concise summary of the removed debug and info logging statements in the tests, detail how the changes were tested (including environment and test commands), and mark the merge checklist items accordingly. Ensure the description clearly communicates the scope and verification of the changes. This will improve review efficiency and merge readiness.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bb31da and 307d0f5.

📒 Files selected for processing (1)
  • tests/model_registry/model_catalog/test_custom_model_catalog.py (0 hunks)
💤 Files with no reviewable changes (1)
  • tests/model_registry/model_catalog/test_custom_model_catalog.py
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2025

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/verified', '/lgtm', '/cherry-pick', '/hold', '/wip', '/build-push-pr-image'}

Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

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

/lgtm

@dbasunag dbasunag merged commit 3983e69 into opendatahub-io:main Sep 9, 2025
9 checks passed
@dbasunag dbasunag deleted the mc_address_comments branch September 9, 2025 19:53
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2025

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

mwaykole pushed a commit to mwaykole/opendatahub-tests that referenced this pull request Jan 23, 2026
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