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(java): release process (take 2) APIC-411 #422

Merged
merged 6 commits into from
Apr 25, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Apr 25, 2022

🧭 What and Why

🎟 JIRA Ticket: APIC-411

The first (#411) try had a lot of issues, mainly caching the right files, this should be correct.

🧪 Test

CI

@millotp millotp self-assigned this Apr 25, 2022
@netlify
Copy link

netlify bot commented Apr 25, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit 2b32a16
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/6266b899a74f11000924fb91

@algolia-bot
Copy link
Collaborator

algolia-bot commented Apr 25, 2022

✗ The generated branch has been deleted.

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

@@ -4,7 +4,6 @@
.mtj.tmp/
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should have been unstaged 🤔

@@ -380,12 +380,13 @@ runs:
if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
uses: actions/cache@v2
with:
path: clients/algoliasearch-client-java-2
path: |
clients/algoliasearch-client-java-2/gradle.properties
Copy link
Member

Choose a reason for hiding this comment

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

Since it's the same for all clients, I think we can have a small block for this path only in this file and keep it in the check.yml file

Copy link
Member

Choose a reason for hiding this comment

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

(So you don't have to put it in each block of this file)

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 get it, you want like a $ref ?

Copy link
Member

Choose a reason for hiding this comment

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

I mean L378 for example you can add

    - name: Restore Java client gradle properties
      if: ${{ inputs.job == 'cts' || inputs.job == 'codegen' }}
      uses: actions/cache@v2
      with:
        path: clients/algoliasearch-client-java-2/gradle.properties
        key: |
          ${{ env.CACHE_VERSION }}-${{
          hashFiles(
            'specs/bundled/search.yml',
            'templates/java/**',
            'generators/src/**'
          )}}

So you don't have to put it in every Java blocks in action.yml

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will that work with the matrix ? because we also call the java cache here

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 think it should work, you can leave the matrix as is

On the matrix: every steps will cache gradle.properties
On the cache: we only restore clients cache, the gradle.properties have its own block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cool, I tried it here b4f83db

Copy link
Member

Choose a reason for hiding this comment

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

Here it's removed, I think the matrix still needs to cache it

The goal is just to make it the cache file less verbose/conditional but idk if it works

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 think the cache actions will still try to execute for that block if I put job == client && language == java also

@@ -98,7 +98,7 @@ jobs:
id: cache
uses: actions/cache@v2
with:
path: ${{ matrix.client.bundledPath }}
path: ${{ format('specs/bundled/{0}.yml', matrix.client.name) }}
Copy link
Member

Choose a reason for hiding this comment

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

I still prefer manipulations in the TS file 😇

Copy link
Member

Choose a reason for hiding this comment

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

Okiii will do it later :(

@@ -1,10 +1,8 @@
plugins {
id 'java-library'
id 'maven-publish'
id 'com.vanniktech.maven.publish'
Copy link
Member

Choose a reason for hiding this comment

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

I thought it was my last name ahaha

matchedGenerator.api = `${clientName}Client`;
matchedGenerator.capitalizedName = clientName;
}
matchedGenerator.config = `${clientName}Config`;
Copy link
Member

Choose a reason for hiding this comment

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

This one is not relevant for clients other than PHP but not a big deal

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the gaol is to simplify a maximum, and remove all edge cases, it's not important if we have to much data passed to the yaml

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.

That looks niiiiiiiiice ಠ_ಠ

@@ -98,7 +98,7 @@ jobs:
id: cache
uses: actions/cache@v2
with:
path: ${{ matrix.client.bundledPath }}
path: ${{ format('specs/bundled/{0}.yml', matrix.client.name) }}
Copy link
Member

Choose a reason for hiding this comment

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

Okiii will do it later :(

.github/workflows/check.yml Outdated Show resolved Hide resolved
@millotp millotp merged commit 32416dc into main Apr 25, 2022
@millotp millotp deleted the feat/java-release-comeback branch April 25, 2022 15:23
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