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

fix(java): use addAlgoliaAgent #652

Merged
merged 7 commits into from
Jun 8, 2022
Merged

fix(java): use addAlgoliaAgent #652

merged 7 commits into from
Jun 8, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Jun 8, 2022

🧭 What and Why

As discussed, we already have the option to override the user-agent by setting the header, so I removed the method setAlgoliaAgent in favor of addAlgoliaAgent and removeAlgoliaAgent that add and remove segments directly.

Also removed the generated from ApiClient.java to make it fully hand written.

🧪 Test

CI

@millotp millotp self-assigned this Jun 8, 2022
@netlify
Copy link

netlify bot commented Jun 8, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit eb8d24b
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62a0b9ebb6b3090009cca762

@algolia-bot
Copy link
Collaborator

algolia-bot commented Jun 8, 2022

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Small question!

@millotp millotp requested a review from shortcuts June 8, 2022 14:22
Base automatically changed from chore/simplify-hook to main June 8, 2022 14:46
shortcuts
shortcuts previously approved these changes Jun 8, 2022
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Looks great, nice!

Comment on lines 32 to 34
if (e.exitCode === 128) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a small log so we know it happened somewhere, is it for the Netlify issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is netlify providing the var env CI=true ? I think it was another unrelated issue, but I can add the log

Copy link
Member

Choose a reason for hiding this comment

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

They have NETLIFY true yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

netlify should never call this pre-commit hook anyway

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

👁️ 👄 👁️ nice!

@millotp millotp enabled auto-merge (squash) June 8, 2022 15:12
@millotp millotp merged commit cbd81a2 into main Jun 8, 2022
@millotp millotp deleted the fix/java-useragent branch June 8, 2022 15:18
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