-
Notifications
You must be signed in to change notification settings - Fork 96
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
[#617]User defined deserializer always should be used before standard… #619
base: master
Are you sure you want to change the base?
Changes from 60 commits
444d929
170103e
8061172
429bf76
97f708f
f0eff31
819d38f
2261231
c63bdf6
526f98e
c92319f
efc574e
87fce39
4e5a0a1
e5346b3
d3e84ec
d54416c
9c1a95f
2e9bfe2
6fb1442
062a161
5703629
38e4808
d4c18ee
7915430
451246a
d5f3bac
ae4663c
a8edee5
84f05f1
f660a9f
0a24b25
0584477
83af633
1cfc5b6
57d2a81
c90d1aa
2efa478
40db71e
339043a
44c7b73
3c10922
e725088
d5daea7
0d8db8e
fe4afe0
a2966bb
0d25125
029e5bc
2d875ef
ee22e55
b5be735
795b5db
c4cd8d4
375eb1f
ac9da78
ef0ccdd
013237c
e836734
b080c99
ef7da0a
e6054b2
fa1f4e1
0574ee2
d167fd7
ceabcf0
49018c1
5ad157e
d287108
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,10 @@ jobs: | |
java_version: [ 11, 17 ] | ||
|
||
steps: | ||
- name: Set up Maven | ||
uses: stCarolas/[email protected] | ||
with: | ||
maven-version: 3.9.5 | ||
- name: Checkout for build | ||
uses: actions/[email protected] | ||
with: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,4 @@ | |
.idea/ | ||
.settings/ | ||
/.DS_Store | ||
/.sdkmanrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright (c) 2015, 2020 Oracle and/or its affiliates. All rights reserved. | ||
* Copyright (c) 2015, 2023 Oracle and/or its affiliates. All rights reserved. | ||
* | ||
* This program and the accompanying materials are made available under the | ||
* terms of the Eclipse Public License v. 2.0 which is available at | ||
|
@@ -22,6 +22,9 @@ | |
*/ | ||
public class JsonBindingProvider extends JsonbProvider { | ||
|
||
public JsonBindingProvider() { | ||
} | ||
|
||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is not needed, since it is added by compile by default There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I wrote in commit comment this is because of JPMS: these are the classes in the root package, which is exported. So I get warnings from the compiler, which suggests adding an explicit constructor. I think the public one is OK, because we should be able to create the instances of these classes outside the packages? |
||
@Override | ||
public JsonbBuilder create() { | ||
return new JsonBindingBuilder(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be removed, if it is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I addressed your approach of making the multi-release JAR, for java pre-16 and above. So the Java 16 sources are now included by plugin and not by manipulation of the compiler plugin configuration. This worked only for test classes but not for main classes; therefore I commended this part out.
The question is: would we like to get the multi-release somehow through some official / right way (there are some now)? Or should we forget it?
I don't think that you got multi-release JAR before, just the compilation and creation of the different JARs for different versions. So maybe this multi-release wasn't a goal anyway? And shouldn't be reached?
As about this comment - I can remove it independently of the decision about the above topic.