Skip to content

chore(ci): add submodules and update release process #221

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

Merged
merged 84 commits into from
Mar 3, 2022
Merged

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Mar 2, 2022

🧭 What and Why

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

Changes included:

  • Adds clients as submodules
  • Process release after the release issue is approved & merged

eunjae-lee and others added 30 commits February 23, 2022 15:43
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.

I looks nice!! GG

First pass, I have a few questions:

  • Can we list somewhere what are the manual/automated parts of the submodules/releases?
  • Should we push generated code?
  • Will it be generated by the CI?
  • In whose name the code is pushed?

@@ -0,0 +1,21 @@
name: Submodule

description: Update submodules
Copy link
Member

Choose a reason for hiding this comment

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

Can we add here why we need this action?


inputs:
token:
description: GitHub Token
Copy link
Member

Choose a reason for hiding this comment

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

Do we need specific ACLs?

@@ -0,0 +1,9 @@
[submodule "clients/algoliasearch-client-java-2"]
Copy link
Member

Choose a reason for hiding this comment

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

Is there a branch to specify somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

ah ok it's in the config, make sense

@@ -0,0 +1,77 @@
## Add a submodule
Copy link
Member

Choose a reason for hiding this comment

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

I think the doc PR will be merged before this one, you can maybe prepare a second branch based on it to avoid merge conflicts

Comment on lines 3 to 10
```sh
git submodule add [email protected]:algolia/algoliasearch-client-javascript.git clients/algoliasearch-client-javascript

(cd clients/algoliasearch-client-javascript && git checkout -b next)

git add clients/algoliasearch-client-javascript
git commit -m "chore: add submodule"
```
Copy link
Member

Choose a reason for hiding this comment

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

We should have the usage with placeholders (e.g.: git submodule add [email protected]:algolia/<YOUR_API_CLIENT_REPOSITORY>.git) first and then a concrete example

Then modify something and push to its origin.

```sh
vi README.md
Copy link
Member

Choose a reason for hiding this comment

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

:yes: vi > code :yes:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot believe I actually wrote vi 😂 (didn't realize it until now)

Comment on lines +12 to +15
"gitAuthor": {
"name": "api-clients-bot",
"email": "[email protected]"
}
Copy link
Member

Choose a reason for hiding this comment

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

is it the author of the release only? Who pushes to the submodules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

process-release.ts does all the commits and pushes.

@@ -65,6 +65,18 @@ export function splitGeneratorKey(generatorKey: string): Generator {
return { language, client, key: generatorKey };
}

export function getGitHubUrl(lang: string): string {
const entry = Object.entries(openapitools['generator-cli'].generators).find(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const entry = Object.entries(openapitools['generator-cli'].generators).find(
const [key, generatorOptions] = Object.entries(openapitools['generator-cli'].generators).find(

or whatever the name is would make it clearer than entry imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not practically, but technically entry can be nullable.

@netlify
Copy link

netlify bot commented Mar 2, 2022

✔️ Deploy Preview for api-clients-automation canceled.

🔨 Explore the source changes: 73391de

🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/6220960df53f9400078a4353

@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Mar 2, 2022

  • Can we list somewhere what are the manual/automated parts of the submodules/releases?

I will create a new PR to update the documentation.

  • Should we push generated code?

No

  • Will it be generated by the CI?

Yes

  • In whose name the code is pushed?

It's specified in release.config.json

@eunjae-lee eunjae-lee requested a review from shortcuts March 3, 2022 09:57
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.

Amazing ! Is there a way to see the generated code on each PR ?

`You can run this script only from \`${MAIN_BRANCH}\` branch.`
);
}
if (process.env.SKIP_VALIDATION !== 'true') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you export this variable as boolean from common.ts pls ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I'm supposed to remove this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eunjae-lee
Copy link
Contributor Author

Amazing ! Is there a way to see the generated code on each PR ?

We only generate clients only on release now. But if we want to generate on each PR, then I guess that's another flow we might invest on. (For example, GitHub Action triggered on new PR, generates client, and pushes it to a temp branch, and comments the links in the PR)

@eunjae-lee eunjae-lee requested a review from millotp March 3, 2022 10:25
@eunjae-lee eunjae-lee merged commit a4f644f into main Mar 3, 2022
@eunjae-lee eunjae-lee deleted the chore/release branch March 3, 2022 10:27
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.

We should not merge if we don't want to lose the file history before the end of the investigation

@shortcuts
Copy link
Member

ah shoot

@eunjae-lee
Copy link
Contributor Author

haha, but it's okay. we don't lose the history yet.

@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Mar 3, 2022

I'll come back with another PR to keep the histories properly. To give you an idea:

  1. I will update the each lang repos with proper histories (on their next branch)
  2. I will update the references to them within the monorepo.

then it's done

shortcuts pushed a commit that referenced this pull request Apr 22, 2022
* chore: provide token to checkout submodule

* chore(ci): remove pipe function

* chore(ci): set author identity

* chore(ci): set git config globally

* chore(ci): update monorepo after submodule changes

* chore: update submodules

* chore(ci): fix copy path

* chore: update submodules

* chore: update submodule

* chore(ci): fix copy command

* chore(ci): fix copy

* chore: update submodules

* chore(ci): fix copy command

* chore: update submodules

* chore: update submodules

* chore(ci): update script

* chore: update submodules

* chore: remove dummy submodule

* chore: remove generated clients

* chore(ci): set up real submodules

* chore: re-apply changes

* chore: support real submodules

* chore: update released tag

* docs: update submodules.md

* chore: update reference to js client

* chore: update workspace config

* chore: update issue template

* chore: update issue template

* chore: update submodule after checking out

* chore: fix script

* chore: update reference to js client

* chore: update yarn workspace

* chore: update reference to js client

* docs: update guide

* chore: update submodule

* chore: add log

* fix: fix clientPath

* add log

* add log

* chore: add log

* chore: try something else

* chore: test

* chore: remove log

* chore: remove javascript repo from submodule

* chore: add javascript repo as submodule again

* chore: update reference to js repo

* chore: update reference to submodules

* chore: fix changelog path

* chore: configure git author in submodules before commiting

* chore: add debug code

* chore: fix client path

* chore: make it fail-safe

* chore: fetch tag before updating it

* chore: update reference to submodule

* chore: fix GHA

* chore: fix broken GHA

* chore: provide token as env var

* chore: move submodule related logic out of action.yml

* chore: extract as actions/submodule

* chore: add missing properties

* chore: fix submodule action

* chore: adding composite

* chore: remove unused env var

* chore: update reference to submodule

* docs: update submodules.md

* chore: update description of GHA

* chore: fix GHA lint error

* chore: fix yaml lint error

* chore: remove debugging code

Co-authored-by: api-clients-bot <[email protected]>
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