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

Update REST client dependency names after extension name change as the Test Suite compilation fails #1617

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Jan 17, 2024

Summary

see https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.7#rest-client-gear-white_check_mark

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@rsvoboda rsvoboda requested review from rsvoboda and mjurc January 17, 2024 15:43
@rsvoboda
Copy link
Member

Where did you see the failures?

There should be relocations in place for changed GAVs, CI is building Quarkus main with relocations:

We are processing 3.2.10 TD, so I'd rather wait with this change till 3.2.10 is processed.
Plus the name change shouldn't by breaking one thanks to relocations.

@michalvavrik
Copy link
Member Author

Where did you see the failures?

locally

We are processing 3.2.10 TD, so I'd rather wait with this change till 3.2.10 is processed.

3.2.10 has it's own branch, I don't think it's sound argument as if you do a backport, you can just tweak one name. I'll respect if it's important for you

Plus the name change shouldn't by breaking one thanks to relocations.

I never needed relocations till now. There is not a single word about them in https://github.com/quarkus-qe/quarkus-test-suite/blob/main/README.md so it is breaking change for me. I can adapt, but disagree.

@rsvoboda
Copy link
Member

3.2.10 - there will be backports from main to 3.2 branch

locally - you need to enable relocations module ... e.g. mvnd clean install -Dquickly -Prelocations -Dno-test-modules

relocations - https://github.com/quarkusio/quarkus/tree/main/relocations
They are used quite a lot, especially when extensions move to quarkiverse

@michalvavrik
Copy link
Member Author

3.2.10 - there will be backports from main to 3.2 branch

locally - you need to enable relocations module ... e.g. mvnd clean install -Dquickly -Prelocations -Dno-test-modules

ok

relocations - https://github.com/quarkusio/quarkus/tree/main/relocations They are used quite a lot, especially when extensions move to quarkiverse

I don't see in Quarkus README or CONTRIBUTING documents any suggestion to use relocation. I don't mind using them based on migration note for now, but if there is such a requirement for our project or Quarkus snapshot, it should be documented.

@rsvoboda
Copy link
Member

it should be documented

+1, please create GH issue in Quarkus repo

@michalvavrik
Copy link
Member Author

it should be documented

+1, please create GH issue in Quarkus repo

the issue should be created by someone who believes they should be used :-) I don't.

Let's conclude this, I'll use relocation and thank you for comments and information.

@rsvoboda rsvoboda mentioned this pull request Jan 19, 2024
9 tasks
Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

@maxandersen
Copy link

it should be documented

+1, please create GH issue in Quarkus repo

the issue should be created by someone who believes they should be used :-) I don't.

Let's conclude this, I'll use relocation and thank you for comments and information.

If you want to have a branch that works with both 3.2 and 3.7-8+ you'll need to have a build with relocations or a profile to handle the diff.

I'm going to check with gsmet why we don't just always build with them enabled and just have a way to turn them of so a build is as close to what users will experience.

@maxandersen
Copy link

conclusion on why we have it and why it is relevant: quarkusio/quarkus#38274 (comment)

@michalvavrik michalvavrik removed the request for review from mjurc January 23, 2024 19:01
@michalvavrik michalvavrik merged commit b670409 into quarkus-qe:main Jan 23, 2024
7 checks passed
@michalvavrik michalvavrik deleted the feature/fix-compilation-after-rest-client-renaming branch January 23, 2024 19:01
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