Skip to content

Conversation

mahajanadhitya
Copy link
Contributor

No description provided.

@cla-assistant
Copy link

cla-assistant bot commented Aug 7, 2023

CLA assistant check
All committers have signed the CLA.

@mahajanadhitya mahajanadhitya force-pushed the feature/AdminClient-ListOffsets branch from 1cb8861 to 6592b2d Compare October 12, 2023 11:44
@mahajanadhitya mahajanadhitya force-pushed the feature/AdminClient-ListOffsets branch from 6592b2d to 7c049ac Compare October 13, 2023 09:25
@emasab emasab force-pushed the feature/AdminClient-ListOffsets branch from ee6da94 to dcf4499 Compare October 18, 2023 11:38
@emasab emasab force-pushed the feature/AdminClient-ListOffsets branch from 23c898a to b5ba287 Compare October 19, 2023 11:04
Copy link
Contributor

@anchitj anchitj left a comment

Choose a reason for hiding this comment

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

Thanks for moving the API to the extensions class, completely forgot about that. Nitpick comments.

Comment on lines +72 to +75
internal TimestampSpec(long timestamp)
{
Timestamp = timestamp;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably keep private set; accessor replacing this.


namespace Confluent.Kafka.Admin
{
/// <summary>
/// Represents the result of a list offsets operation.
/// Represents an error that occurred during a ListOffsets request.
Copy link
Contributor

Choose a reason for hiding this comment

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

The result of ListOffsets request (including error status).

}
};

var listOffsetsResult = await adminClient.ListOffsetsAsync(topicPartitionOffsetSpecs,options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Spacing

Copy link
Contributor

@anchitj anchitj left a comment

Choose a reason for hiding this comment

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

LGTM!

@emasab emasab merged commit 07de95e into master Oct 19, 2023
@emasab emasab deleted the feature/AdminClient-ListOffsets branch October 19, 2023 17:38
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.

3 participants