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

feat(generators): factor utilsPackageVersion for JavaScript #511

Merged
merged 3 commits into from
May 18, 2022

Conversation

shortcuts
Copy link
Member

🧭 What and Why

🎟 JIRA Ticket: -

Changes included:

When generating a new JavaScript client, the default utilsPackageVersion will match the client version (e.g. #485 tries to fetch 0.0.1 of utils)

This PR aims at applying the same logic added for packageVersion:

  • Define utilsPackageVersion in config/clients.config.json
  • Extend getPackageVersion logic to the whole client config to be able to retrieve other fields
  • Reflect changes in release process script

🧪 Test

CI :D

@shortcuts shortcuts requested review from eunjae-lee and millotp May 18, 2022 11:27
@shortcuts shortcuts self-assigned this May 18, 2022
@netlify
Copy link

netlify bot commented May 18, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit 86fcb3d
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/6284f59d13cc1100086e7060

@shortcuts shortcuts force-pushed the feat/utilsPackageVersion-factor branch from baa4ce3 to 840a703 Compare May 18, 2022 11:28
@algolia-bot
Copy link
Collaborator

algolia-bot commented May 18, 2022

✗ The generated branch has been deleted.

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

@shortcuts
Copy link
Member Author

I tried #485 locally with a rebase it seems to work, but you can rebase your branch to ensure it @bodinsamuel

Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

Nice ! The only field left is hasRegionalHost, will be a bit more tricky without a lot of copy pasting :)

scripts/config.ts Outdated Show resolved Hide resolved
scripts/release/process-release.ts Outdated Show resolved Hide resolved
scripts/config.ts Outdated Show resolved Hide resolved
@shortcuts shortcuts force-pushed the feat/utilsPackageVersion-factor branch from 8a2c02f to 8b92804 Compare May 18, 2022 13:15
@shortcuts shortcuts force-pushed the feat/utilsPackageVersion-factor branch from 8b92804 to 86fcb3d Compare May 18, 2022 13:33
import type { Language } from './types';
import type { Language, LanguageConfig } from './types';

export function getClientsConfigField(
Copy link
Collaborator

Choose a reason for hiding this comment

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

ahah how to overcomplicate thing, I agree that this is needed for Java but for ts we can just read json as is, but make sense for error checking

Copy link
Member Author

Choose a reason for hiding this comment

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

Offers a way to avoid having a method for each field as @eunjae-lee suggested and also throws for all cases :D

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.

4 participants