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.

Sign up for GitHub

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

Jump to bottom

feat(generator): add custom generator to fix customRequest #315

Merged
merged 7 commits into from
Mar 31, 2022
Merged

feat(generator): add custom generator to fix customRequest #315

merged 7 commits into from
Mar 31, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Mar 31, 2022 •
edited
Loading

🧭 What and Why

To fix PR #313, adding custom generator for JavaScript named algolia-javascript, and PHP named algolia-php

Also includes generators changes in the CI will invalidate cache,
and add generator as a PR title scope.

🧪 Test

CI

@millotp millotp requested a review from a team March 31, 2022 09:18
@millotp millotp self-assigned this Mar 31, 2022
@millotp millotp requested review from eunjae-lee and shortcuts and removed request for a team March 31, 2022 09:18
@netlify
Copy link

netlify bot commented Mar 31, 2022 •
edited
Loading

✅ Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit 1659bed
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/62458233f766210008de995c

@shortcuts
Copy link
Member

To fix #313,

Be careful with the fix + PR number, it will close my PR D:

@algolia-bot
Copy link
Collaborator

algolia-bot commented Mar 31, 2022 •
edited
Loading

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the generated/main branch.

@millotp
Copy link
Collaborator Author

millotp commented Mar 31, 2022

What is this feature ahah, totally random

@shortcuts
Copy link
Member

What is this feature ahah, totally random

It's because PRs are issues, so it works for both

@millotp
Copy link
Collaborator Author

millotp commented Mar 31, 2022

Fastest CI ever, but it did not run correctly

@shortcuts
Copy link
Member

Fastest CI ever, but it did not run correctly

We should consider generator changes like template changes now

shortcuts
shortcuts reviewed Mar 31, 2022
View reviewed changes
Copy link
Member

@shortcuts shortcuts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! Small comment

thanks :DDD

@millotp millotp changed the title feat(javascript): add custom generator to fix customRequest feat(generator): add custom generator to fix customRequest Mar 31, 2022
shortcuts
shortcuts previously approved these changes Mar 31, 2022
View reviewed changes
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 good! Thanks for making it quick :D

config/clients.config.json
@@ -1,7 +1,7 @@
{
"java": {
"folder": "clients/algoliasearch-client-java-2",
"mainGenerator": "java-search",
"mainPackage": "java-search",
Copy link
Member

Choose a reason for hiding this comment

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

Not needed change right now IMO but tagging @eunjae-lee for visiblity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't want the confusion with customGenerator and mainPackage, they are completely unrelated.

Copy link
Member

Choose a reason for hiding this comment

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

Yep I totally agree, just stating that it's out of the scope of this PR, but not a huge deal here

@millotp millotp enabled auto-merge (squash) March 31, 2022 10:07
shortcuts
shortcuts approved these changes Mar 31, 2022
View reviewed changes
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.

Looking good!

@millotp millotp merged commit 59ce8b4 into main Mar 31, 2022
@millotp millotp deleted the feat/js-custom-gen branch March 31, 2022 10:38
@shortcuts shortcuts mentioned this pull request Apr 4, 2022
Merged
shortcuts pushed a commit that referenced this pull request Apr 22, 2022
@millotp
feat(generator): add custom generator to fix customRequest (#315)
5b79563
This was referenced May 24, 2022
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shortcuts shortcuts shortcuts approved these changes

@eunjae-lee eunjae-lee Awaiting requested review from eunjae-lee eunjae-lee is a code owner automatically assigned from algolia/api-clients-automation

Assignees

@millotp millotp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@millotp @shortcuts @algolia-bot