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(javascript): update template to include correct dependency version #328

Merged
merged 9 commits into from
Apr 5, 2022

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Apr 4, 2022

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-407

Changes included:

  • It updates the template to have the correct version
  • It updates the command to pin the dependencies

@netlify
Copy link

netlify bot commented Apr 4, 2022

Deploy Preview for api-clients-automation failed.

Name Link
🔨 Latest commit 8a5cf42
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/624c2f8ec77189000824cde3

@algolia-bot
Copy link
Collaborator

algolia-bot commented Apr 4, 2022

✗ The generated branch has been deleted.

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

@eunjae-lee eunjae-lee requested review from a team, damcou and shortcuts and removed request for a team April 4, 2022 13:13
@@ -31,7 +32,10 @@ export const GENERATORS: Record<string, Generator> = {
key: 'javascript-algoliasearch',
additionalProperties: {
packageName: '@experimental-api-clients-automation/algoliasearch',
packageVersion: '0.0.5',
packageVersion:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We pull the version from the main package (in case of js, it's client-search).

Copy link
Member

@shortcuts shortcuts Apr 4, 2022

Choose a reason for hiding this comment

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

We could also import it from algoliasearch's package.json file directly, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we need to assume openapitools.json and package.json files are always in sync at any point of time. But I'm not sure if we can make such an assumption. It's easier to maintain if we take openapitools.json as a single and only source of truth. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

package.json depends on openapitools.json, so we know that the package.json will be up to date when changes happen in the main file, but once we remove clients from main we won't have this package.json actually so let's go with your solution

@eunjae-lee eunjae-lee enabled auto-merge (squash) April 5, 2022 12:01
@eunjae-lee eunjae-lee merged commit ebafcf3 into main Apr 5, 2022
@eunjae-lee eunjae-lee deleted the fix/template branch April 5, 2022 12:13
@eunjae-lee eunjae-lee mentioned this pull request Apr 5, 2022
2 tasks
algolia-bot added a commit to algolia/algoliasearch-client-javascript that referenced this pull request Apr 5, 2022
This was referenced Apr 5, 2022
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