Skip to content

Conversation

@Soap-141
Copy link
Contributor

@Soap-141 Soap-141 commented Jun 26, 2025

GitHub Issue: #

Proposed Changes

  • Bug fix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes, no api changes)
  • Build or CI related changes
  • Documentation content changes
  • Other, please describe:

What is the current behavior?

The 'IReviewService.TryRequestReview' doesn't return a result.

What is the new behavior?

The 'IReviewService.TryRequestReview' now returns a result.

Impact on version

  • Major (Public API was modified.)
    • Public constructs (class, struct, delegate, enum, etc.) were removed or renamed.
    • Public members were removed or renamed.
    • Public method signatures were changed or renamed.
  • Minor (Public API was extended.)
    • Public constructs, members, or overloads were added.
  • Patch (Public API was unchanged.)
    • A bug in behavior was fixed.
    • Documentation was changed.
  • None (The library is unchanged.)
    • Only code under the build folder was changed.
    • Only code under the .github folder was changed.

Checklist

Please check that your PR fulfills the following requirements:

  • Documentation has been added/updated.
  • Automated Unit / Integration tests for the changes have been added/updated.
  • Updated BREAKING_CHANGES.md (if you introduced a breaking change).
  • Your conventional commits are aligned with the Impact on version section.

Other information

@Soap-141 Soap-141 changed the title Dev/thla/dat feat!: Prompt Request Result Jun 26, 2025
@Soap-141 Soap-141 force-pushed the dev/thla/dat branch 2 times, most recently from 9a97808 to 9fab576 Compare June 26, 2025 14:14
@Soap-141 Soap-141 marked this pull request as ready for review June 26, 2025 14:16
@Soap-141 Soap-141 requested a review from Copilot June 26, 2025 14:29
@Soap-141 Soap-141 self-assigned this Jun 26, 2025
Copy link

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 makes the review service’s TryRequestReview method return structured results and updates all consumers and tests accordingly.

  • Changed TryRequestReview to return a ReviewRequestResult containing status and success info
  • Added ReviewRequestResult and the ReviewPromptStatus enum (with mapping extension)
  • Updated native prompters, interfaces, tests, docs, and bumped relevant package versions

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/ReviewService/ReviewService.cs Changed TryRequestReview signature and logging to return a result
src/ReviewService.Abstractions/ReviewRequestResult.cs Added new ReviewRequestResult type
src/ReviewService.Abstractions/ReviewPromptStatus.cs Introduced ReviewPromptStatus enum
src/ReviewService.Abstractions/IReviewService.cs Updated interface to return ReviewRequestResult
src/ReviewService.NativePrompters/ReviewStatus.Extensions.cs Added mapping from native ReviewStatus to ReviewPromptStatus
src/ReviewService.Tests/ReviewServiceShould.cs Refactored tests to assert on new return type and use NSubstitute
README.md Updated example usage to handle the new result type
BREAKING_CHANGES.md Documented breaking change for TryRequestReview
Comments suppressed due to low confidence (2)

README.md:66

  • There's an extra space between '(' and '$' in the interpolated string, which will not compile. It should be Console.WriteLine($"Review request status: {result.Status}.");.
              Console.WriteLine( $ "Review request status: {result.Status}.");

src/ReviewService.Tests/ReviewServiceShould.cs:262

  • [nitpick] Consider verifying that _reviewPrompterMock.TryPrompt() was invoked exactly once when conditions are satisfied in this test to ensure the prompter is exercised correctly.
		Assert.False(result.IsSuccessful);

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.

4 participants