Skip to content

[MNG-8676] Improve model builder error messages #2257

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 8 commits into from
Apr 16, 2025

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Apr 13, 2025

Only the one that is easy to be mixed up with GAV: DepMgtKey is GAT and not GAV. The rest (XML, PK/GA, REPOID, DIR, GAV) ones are not quite mixable, and string "A:B:C" is usually always assumed is GAV.

The problem with these two are that they look pretty much same (both a "A:B:C", colon segmented string of 3 or 4), but in one case C is dependency type, in other is version.

This PR now lessens the "A:B:C" string overload a bit, and also makes crystal clear that T is not V. The message is precise, but a bit longer. In turn, is not misleading at all now.


https://issues.apache.org/jira/browse/MNG-8676

Only the two that are easy to be mixed up: GAV and GAT related,
as the rest (XML, PK/GA, REPOID, DIR) ones are not quite mixable.

The problem with these two are that they look pretty much same
(both a "A:B:C", colon segmented string of 3 or 4), but in one case
C is dependency type, in other is version.

---

https://issues.apache.org/jira/browse/MNG-8676
@cstamas cstamas self-assigned this Apr 13, 2025
@cstamas cstamas added this to the 4.0.0-rc-4 milestone Apr 13, 2025
@cstamas cstamas marked this pull request as ready for review April 13, 2025 19:35
@cstamas
Copy link
Member Author

cstamas commented Apr 14, 2025

Am unsure about this PR. The one and only reason why it exists is explained in issue, but in short, the "a:b:c" string is usually implied to be GAV, but is also overloaded in Maven as, while many times it is GAV, sometimes is not, but muscle reflex will read it as GAV, and this will cause issues and derailment later on.

Maybe all we need is "just" change the syntax in case of GAT to something else? Like G:A:V vs G:A@T in the message only? To make it visually apparent "is not GAV"?

@gnodet
Copy link
Contributor

gnodet commented Apr 14, 2025

Am unsure about this PR. The and and only reason why it exists is explained in issue, but in short, the "a:b:c" string is usually implied to be GAV, but is also overloaded in Maven as, while many times it is GAV, sometimes is not, but muscle reflex will read it as GAV, and this will cause issues and derailment later on.

Maybe all we need is "just" change the syntax in case of GAT to something else? Like G:A:V vs G:A@T in the message only? To make it visually apparent "is not GAV"?

It would make sense to make sure we use a single syntax everywhere. Not sure that G:A@T would be appropriate, as we sometime use the full syntax G:A:V:T:E I think. So in this case, maybe G:A::T

Another problem would be goals...

@cstamas
Copy link
Member Author

cstamas commented Apr 14, 2025

My only and solely goal is to make visually clear when it is GAV and when it is GAT... and no, if there is T there is never E. So you might mean GAVCT?

This is another thing: Resolver "string" (GAECV) vs Maven "string" (GATCV) and many variations on it....
https://github.com/apache/maven-resolver/blob/e2686c1eed535ee006b8b9cd6787126c3f0a5899/maven-resolver-api/src/main/java/org/eclipse/aether/artifact/DefaultArtifact.java#L55-L56
vs

@cstamas cstamas merged commit 3ba9489 into apache:master Apr 16, 2025
13 checks passed
@cstamas cstamas deleted the MNG-8676 branch April 16, 2025 09:43
@jira-importer
Copy link

Resolve #9632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants