Panache: Deprecate Parameters in favour of Map.of

[FroMage]

Well that's 1h of my time I'll never get back.

• Closes: Panache Next: passing Parameters.with() to find() does not work #52543

[github-actions]

🙈 The PR is closed and the preview is expired.

[loicmathieu]

Please deprecate for real all methods and not only via doclet

[gsmet]

BTW, deprecating is fine with me but shouldn't you make it work even though it's deprecated? At least if it's not requiring an enormous amount of effort?

[FroMage]

BTW, deprecating is fine with me but shouldn't you make it work even though it's deprecated? At least if it's not requiring an enormous amount of effort?

Make it work where? This never stopped working. I don't even intend to remove it.

[geoand]

The linked issue mentions that it didn't work

[FroMage]

The linked issue mentions that it didn't work

There's confusion here. Parameters were never added to Panache Next. It was never in the API, and I'm not going to add (deprecated) support for it in the API.

The user tried to use it in the Panache Next API, and given that the API accepts Object... for positional parameters, it was accepted by the typechecker unnoticed.

In order to "fix" this, I'd have to add the deprecated method just to catch that some user is trying an API that never existing based on the previous versions of Panache which are not compatible in the first place.

I mean, I could, but that's a quite huge stretch to expect such safe-guards. It's a different API entirely.

I think it's reasonable to make the type (which is visible from Panache Next) and methods (from Panache 1) deprecated, because users would notice that they should not be using that type. Even though it does impact Panache 1 users for no especially good reason (I don't see the value in removing this type or these methods, especially for an API that will end up deprecated entirely).

[geoand]

Fair, but couldn't we detect that and print a better error message?

[FroMage]

We could detect this with scanning source code, or bytecode, or an APT processor… none of which sound like the right way to fix this kind of user error.

[geoand]

But don't you think that users will assume that thr use of Parameters is valid given their experience with Panache 1?

[FroMage]

They could, but the minute they try, it's big red and deprecated…

You really think I should add either the deprecated methods, or some bytecode sniffer, to stop the build when someone writes bogus code? I don't mean to imply it'd be crazy, I'm just surprised people would suggest this, so it's giving me pause to think.

I plan to remove and rewrite more methods from PanacheRepository, perhaps I did not change enough that people think it's the same API and I should have made it more different from the start 🤔

[geoand]

My point is that Panache Next is barely out and people already tried to use class in question and didn't understand why it didn't work.
It may be accidental, but I wouldn't bet on it

[geoand]

This will also need a rebase onto main to pick up a CI fix

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 3415a04.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 3415a04.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[FroMage]

I think we can start with this, and reconsider if people complain.

[loicmathieu]

LGTM