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(java): release package with name algoliasearch-client-java #498

Merged
merged 1 commit into from
May 12, 2022

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented May 12, 2022

🧭 What and Why

The name of the package was incorrect, use this to rename to everything to algoliasearch-client-java,

Changes included:

  • Rename folder algoliasearch-client-java-2 to algoliasearch-client-java (by hand to not remove files)

🧪 Test

playground or CTS

@millotp millotp self-assigned this May 12, 2022
@netlify
Copy link

netlify bot commented May 12, 2022

Deploy Preview for api-clients-automation canceled.

Name Link
🔨 Latest commit ae4d23b
🔍 Latest deploy log https://app.netlify.com/sites/api-clients-automation/deploys/627d26de6a8285000887b7b1

@millotp millotp marked this pull request as draft May 12, 2022 09:57
@algolia-bot
Copy link
Collaborator

algolia-bot commented May 12, 2022

✗ The generated branch has been deleted.

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

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 don't think the folder rename is necessary, it just matches the name of the repository it will be hosted, and we plan to remove code from here later anyway.

If you plan to change the repo of destination, I'd suggest you to talk about this with the team, as there might be reasons why we've used the java-2 instead of -java

"folder": "clients/algoliasearch-client-java-2",
"packageVersion": "0.0.1",
"folder": "clients/algoliasearch-client-java",
"packageVersion": "4.0.0",
Copy link
Member

@shortcuts shortcuts May 12, 2022

Choose a reason for hiding this comment

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

We should maybe not bump this right now, otherwise if you release people will see it and try to use it, no?

(idk how it works on sonatype etc I'm maybe wrong)

Base automatically changed from chore/factor-version to main May 12, 2022 10:01
@millotp
Copy link
Collaborator Author

millotp commented May 12, 2022

I've talked with Mouaad about this, the folder name here is irrelevant and the 2 doesn't make sense.
I don't plan on renaming the repo name, as we have the variable gitRepoId for this.
This is more about renaming the jar that will be accessible

@shortcuts
Copy link
Member

I've talked with Mouaad about this, the folder name here is irrelevant and the 2 doesn't make sense.

But it matches the repo of destination, otherwise we could have much simpler names.

It's less confusing like that, so you know it has no relation with https://github.com/algolia/algoliasearch-client-java

I don't plan on renaming the repo name, as we have the variable gitRepoId for this.

gitRepoId also changed in this PR

@millotp
Copy link
Collaborator Author

millotp commented May 12, 2022

gitRepoId also changed in this PR

that's why it's a draft ahah I'm not done

@millotp millotp marked this pull request as ready for review May 12, 2022 14:04
@millotp millotp requested review from a team, eunjae-lee and shortcuts and removed request for a team May 12, 2022 14:04
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.

IIRC it can also break things to move > re-move, I'd suggest you to squash + rebase D:

@@ -0,0 +1 @@
POM_ARTIFACT_ID=algoliasearch-client-java
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it be algoliasearch-core if we have multiple package under this repository later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's not planned for now, I'm not sure what the correct package should be called but it won't be algoliasearch-core for now

Copy link
Member

Choose a reason for hiding this comment

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

Ideally it should be the exact same as today, to avoid losing version history etc. but during this experimental phase it can be whatever indeed.

What do you think of adding the experimental like we did for JS? So it's a bit of warning and less confusing to users

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SNAPSHOT is the same as experimental for user, they cannot see it and have to add a special repository by hand to be able to use it. It's like another package.

Copy link
Member

Choose a reason for hiding this comment

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

Ah oki make sense then!

"groupId": "com.algolia",
"apiPackage": "com.algolia.api",
"invokerPackage": "com.algolia",
"modelPackage": "com.algolia.model.search",
"library": "okhttp-gson",
"gitRepoId": "algoliasearch-client-java-2",
"additionalProperties": {
"packageName": "algoliasearch-client-java-2"
"packageName": "algoliasearch-client-java"
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 it's called algoliasearch in maven in the current version, does it changes something for us to change this name? I don't know how package managers work for Java

Copy link
Member

Choose a reason for hiding this comment

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

ah algoliasearch-core exists too

this will break the version bump, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

packageName is only used for js, I should remove it here but it changes lots of scripts

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.

Looking good!

@millotp millotp merged commit 57f0edd into main May 12, 2022
@millotp millotp deleted the fix/java-release-name branch May 12, 2022 16:33
algolia-bot added a commit to algolia/algoliasearch-client-java that referenced this pull request May 12, 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