Make glassfish json a required dependency again #368
Conversation
etc/delivery-checks.sh
Outdated
| mvn checkstyle:checkstyle && \ | ||
| mvn org.glassfish.copyright:glassfish-copyright-maven-plugin:copyright && \ | ||
| echo "Completed normally" |
There was a problem hiding this comment.
This file is not directly used anywhere in the build pipeline. It's just something I wrote for myself to quickly run the delivery checks locally before opening a PR and I thought I'd share it by checking it in
There was a problem hiding this comment.
OK, well just in that case I would suggest to be aware of that "copyright" goal does only print out warnings about the files and not fails itself. Goal "check" fails, but does not print any warnings. :D
This is fine if you know about this, but if someone will use it and doesn't know that, he might be a bit surprised after push even when he got "Completed normally".
There was a problem hiding this comment.
yea that is not ideal, could we perhaps configure the copyright goal to fail instead of just warn?
There was a problem hiding this comment.
That is the problem. I tried configure it that way, but I am not sure it is possible right now. Maybe I am missing something, but I could not find any correct settings for that. (That is the reason why I do have both goals under "copyright" profile.)
There was a problem hiding this comment.
@Verdent I've updated the PR to run etc/copyright.sh so it will fail if the copyright goal fails 👍
There was a problem hiding this comment.
Sounds good to me. Just beware of the file with results that will this copyright.sh creates. It might be worth removing it when you are done.
There was a problem hiding this comment.
I'll change the copyright.sh script so that it outputs the results to the target/ folder
aguibert
force-pushed
the
glassfish-json-required
branch
from
b5142d5 to
3f67491
Compare
aguibert
force-pushed
the
glassfish-json-required
branch
from
3f67491 to
efd41e6
Compare
While testing out some local changes with 1.0.6-SNAPSHOT, I noticed that it is no longer possible to run a standalone program by simply adding Yasson to my pom.xml. I got errors indicating no JSON-P implementation (Glassfish) was found.
Back in 1.0.5 I thought we had Glassfish marked as an optional dependency, but due to some odd interactions with the old
<dependencyManagement>block it ended up being a required dependency.In order to preserve existing behavior with 1.0.5 and earlier we must add glassfish json back in as a required dependency.
@Verdent please review