Added or Updated XML docs. - #29
Merged
Merged
Conversation
Contributor
Reviewer's GuideAdds and refines XML documentation comments across the LDAP helper library, adapters, DTOs, demo code, and tests to better describe public APIs, behaviors, and usage patterns without changing runtime logic. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new XML docs for NovellLdapMessageAdapter.IsSearchDone describe normal usage, but the method still just throws an exception; consider either implementing the check or updating the docs to reflect that it's not supported yet.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new XML docs for NovellLdapMessageAdapter.IsSearchDone describe normal usage, but the method still just throws an exception; consider either implementing the check or updating the docs to reflect that it's not supported yet.
## Individual Comments
### Comment 1
<location path="src/Bitai.LDAPHelper/Searcher.cs" line_range="45" />
<code_context>
/// Searches for entries matching the provided LDAP filter and loads the requested attributes.
/// </summary>
- /// <param name="searchFilter">
+ /// <param name="searchFilterObject">
/// A combinable LDAP filter that identifies the entries to search for. This filter will be converted
/// to its string representation and used directly in the LDAP search operation.
</code_context>
<issue_to_address>
**issue:** XML doc param name no longer matches the method signature, which will cause documentation/analysis issues.
The `<param>` tag now refers to `searchFilterObject`, but the actual parameter name appears to remain `searchFilter`. This mismatch will trigger XML doc warnings and may mislead consumers using generated docs or IntelliSense. Please align the names by either reverting the doc tag to `searchFilter` or renaming the parameter to `searchFilterObject`.
</issue_to_address>
### Comment 2
<location path="src/Bitai.LDAPHelper.LdapAdapters/ILdapMessageAdapter.cs" line_range="3-12" />
<code_context>
namespace Bitai.LDAPHelper.Demo;
+/// <summary>
+/// Holds runtime context and resolved settings for demo execution.
+/// </summary>
</code_context>
<issue_to_address>
**issue (bug_risk):** Documented semantics for IsSearchDone don’t match the current Novell adapter implementation, which throws an exception.
As written, consumers will expect `IsSearchDone` to be a safe state indicator, but `NovellLdapMessageAdapter.IsSearchDone` still throws a generic `Exception`. This mismatch will cause runtime failures. Please either align the implementation with the documented semantics (e.g., derive the value from `LdapSearchResultDone`) or, if that’s not yet possible, change it to return a safe default instead of throwing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
3
to
+12
| /// <summary> | ||
| /// Target interface for LDAP message operations | ||
| /// Defines accessors for LDAP protocol messages returned by search operations. | ||
| /// </summary> | ||
| public interface ILdapMessageAdapter | ||
| { | ||
| /// <summary> | ||
| /// Gets the LDAP entry when the message represents a search result; otherwise <see langword="null"/>. | ||
| /// </summary> | ||
| ILdapEntryAdapter Entry { get; } | ||
|
|
Contributor
There was a problem hiding this comment.
issue (bug_risk): Documented semantics for IsSearchDone don’t match the current Novell adapter implementation, which throws an exception.
As written, consumers will expect IsSearchDone to be a safe state indicator, but NovellLdapMessageAdapter.IsSearchDone still throws a generic Exception. This mismatch will cause runtime failures. Please either align the implementation with the documented semantics (e.g., derive the value from LdapSearchResultDone) or, if that’s not yet possible, change it to return a safe default instead of throwing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
XML docs updated on all projects.
Summary
Add comprehensive XML documentation across LDAP helper core classes, DTOs, adapters, demo utilities, and tests to clarify responsibilities, parameters, and result semantics.
Enhancements:
Documentation: