Skip to content

chore(ci): update cts and playground dependencies on release #467

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

Closed
wants to merge 1 commit into from

Conversation

eunjae-lee
Copy link
Contributor

🧭 What and Why

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

Changes included:

This PR makes the release process to bump the versions in cts and playground to the locally latest versions. This only works for JavaScript, and for other languages, it should be dealt separately.

@netlify
Copy link

netlify bot commented May 2, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit fcb37be
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/626faa228eeb610008dc189b

@algolia-bot
Copy link
Collaborator

algolia-bot commented May 2, 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

shortcuts commented May 2, 2022

Since we have all the informations of the openapitools.json in the AlgoliaCtsGenerator, we could have this logic done in Java, so it would work for all the clients, wdyt?

It would for example output the package.json file for JavaScript, build.gradle file for Java

edit: ah but we would still need some logic for the playground 🤔 , however CTS gen could output both files at their correct location

@eunjae-lee
Copy link
Contributor Author

Since we have all the informations of the openapitools.json in the AlgoliaCtsGenerator, we could have this logic done in Java, so it would work for all the clients, wdyt?

It would for example output the package.json file for JavaScript, build.gradle file for Java

edit: ah but we would still need some logic for the playground 🤔 , however CTS gen could output both files at their correct location

yeah we "could" probably do that, but it's "CTS" generator. So it feels unrelevant to what we're doing here. We bump versions during the release process, and want to bump all the related packages. So it makes sense to run all the steps within the release process. WDYT?

@shortcuts
Copy link
Member

So it makes sense to run all the steps within the release process. WDYT?

Yep I agree, I just feel like it could be globally handled by the generator instead of inside the release process, with logic for each language.

Since we re-gen everything before a release anyway, we can have this logic in each language generator

@eunjae-lee
Copy link
Contributor Author

So it makes sense to run all the steps within the release process. WDYT?

Yep I agree, I just feel like it could be globally handled by the generator instead of inside the release process, with logic for each language.

Since we re-gen everything before a release anyway, we can have this logic in each language generator

"Since we re-gen everything before a release anyway"

We don't know if we will have this later or not ;)

@shortcuts
Copy link
Member

shortcuts commented May 2, 2022

We don't know if we will have this later or not ;)

How do you handle the version bump without that? It would invalidate openapitools.json being the single source of truth

Anyway I'm fine with this implem for the JavaScript, I just don't want us to have to do file manipulation with regexes etc. for each languages

If we go with that, we should at least have a solution for Java too, I think the bump of testImplementation 'com.algolia:algoliasearch-core:0.0.1-SNAPSHOT' is necessary to run CTS/playground

@eunjae-lee
Copy link
Contributor Author

Without any regex, we can simply find if a line contains testImplementation 'com.algolia:algoliasearch-core: and replace it with a new version.

does yarn generate do anything about cts or playground? I just think it's not a good idea to put something related to version bumping in non-release related place.

@shortcuts
Copy link
Member

does yarn generate do anything about cts or playground?

We bump the versions of the openapitools.json and then generate clients with their new version, it could have some extra logic to update them

@eunjae-lee
Copy link
Contributor Author

does yarn generate do anything about cts or playground?

We bump the versions of the openapitools.json and then generate clients with their new version, it could have some extra logic to update them

hmmm that could work, but then we run the extra logic every time we run generate, even though not in the context of release. it'd make sense if we add an option like yarn generate javascript --update-playground --update-cts or something?

@shortcuts
Copy link
Member

shortcuts commented May 2, 2022

it'd make sense if we add an option like yarn generate javascript --update-playground --update-cts or something?

I guess the easiest to avoid impacting the current CLI usage would be env variable (like RELEASE=true yarn generate or yarn generate --release indeed) that bump playground and cts when specified

This way we can still correctly do a manual release too

@eunjae-lee
Copy link
Contributor Author

it'd make sense if we add an option like yarn generate javascript --update-playground --update-cts or something?

I guess the easiest to avoid impacting the current CLI usage would be env variable (like RELEASE=true yarn generate or yarn generate --release indeed) that bump playground and cts when specified

This way we can still correctly do a manual release too

"release" as a name to the generator means nothing to me. It can be really vague and confusing. And it doesn't represent what the option does.

I'm still not in favor of making the generator do the work of bumping versions.

If you need a way to manually release, then IMO, what we should do is, to create another CLI command to bump the version, and let the release process call this command.

@shortcuts
Copy link
Member

shortcuts commented May 2, 2022

"release" as a name to the generator means nothing to me.

It was an example for the usage

I'm still not in favor of making the generator do the work of bumping versions.

But that's what's done today?

@eunjae-lee
Copy link
Contributor Author

@shortcuts
Copy link
Member

shortcuts commented May 2, 2022

what do you mean? we bump versions in process-release

Yup, we bump the file, then generate, then push, it's exactly what I'm suggesting, but instead of running yarn generate, we could run yarn generate --release (whatever the name is) that triggers the part of the Java generator that also updates the CTS and Playground

@eunjae-lee eunjae-lee marked this pull request as draft May 4, 2022 08:26
@eunjae-lee
Copy link
Contributor Author

continued in #490

@eunjae-lee eunjae-lee closed this May 10, 2022
@millotp millotp deleted the chore/update-vers branch May 20, 2022 14:12
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