Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable okhttp redirects for ctxsvc #1606

Merged
merged 2 commits into from
Mar 17, 2021
Merged

Disable okhttp redirects for ctxsvc #1606

merged 2 commits into from
Mar 17, 2021

Conversation

BenWu
Copy link
Contributor

@BenWu BenWu commented Mar 17, 2021

No description provided.

@BenWu BenWu requested a review from jklukas March 17, 2021 20:39
Copy link
Contributor

@jklukas jklukas left a comment

Choose a reason for hiding this comment

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

Looks good! One suggestion in the test.

Comment on lines 122 to 127
Iterator<PubsubMessage> iterator = messages.iterator();
int messageCount = 0;

for (; iterator.hasNext(); iterator.next()) {
messageCount++;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Iterator<PubsubMessage> iterator = messages.iterator();
int messageCount = 0;
for (; iterator.hasNext(); iterator.next()) {
messageCount++;
}
int messageCount = Iterators.size(messages.iterator());

Slight preference for using a guava helper in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow that's useful

Copy link
Contributor

Choose a reason for hiding this comment

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

Guava has a bunch of helpers like this that can make code prettier. Don't expect any magic performance-wise, though. I'm pretty sure Iterators.size() is doing exactly the same thing you already have written out here.

@BenWu BenWu merged commit ee3784e into master Mar 17, 2021
@BenWu BenWu deleted the benwu/ctxsvc-no-redirect branch March 17, 2021 21:24
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.

None yet

2 participants