Enabling JDK 8 Doclint Checks (#623) #624

mkarg · 2018-04-21T12:18:54Z

Improves quality of API JavaDocs by enabling JDK 8 doclint checks and fixing all WARNINGs and ERRORs (see issue #623).

ggam · 2018-04-22T08:28:45Z

Thanks a lot Markus! I'll take care of the warnings myself.

mkarg · 2018-04-22T11:38:52Z

@ggam Thanks! Can't wait to review your contribution! :-)

ggam · 2018-04-22T11:51:31Z

@mkarg I already made a PR to your branch (I cannot push directly since I'm not a repository member). El dom., 22 abr. 2018 13:38, Markus KARG <notifications@github.com> escribió:

…

@ggam <https://github.com/ggam> Thanks! Can't wait to review your contribution! :-) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#624 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACAucFKWZcSDFTxd2f8V-fxAekjXJGExks5trGvMgaJpZM4Teb5H> .

chkal

I just added a few comments.

BTW: This PR also contains the commits from #622 which is a bit confusing. But I guess the plan is to rebase this after #622 gets merged, correct?

chkal · 2018-04-28T07:18:35Z

jaxrs-api/src/main/java/javax/ws/rs/BeanParam.java

 * For example:
- * <pre>
+ * <PRE>


Just wondering: Are uppercase <pre> tags enforced by doclint? I'm just asking, because all other HTML elements are lowercase.

Strange but true, but on my machine this actually did fix a Doclint complaint. Maybe a bug in Doclint, I don't know really.

chkal · 2018-04-28T07:24:18Z

jaxrs-api/src/main/java/javax/ws/rs/core/HttpHeaders.java

@@ -45,8 +45,8 @@
    public List<String> getRequestHeader(String name);

    /**
-     * Get a HTTP header as a single string value.
-     * <p/>
+     * <p>Get a HTTP header as a single string value.


I may be too niggling here, but IMO we should either put both the start and end element on a line nor non of them.

So either:

 Foobar 

Or:

Foobar

I have no feelings at all about that. My intention was to touch as few lines as possible to make the PR at-most reviewable. I abstained form applying any style at all, because we do not have agreed upon a particular style, and the current Javadocs seems to be chaotic. The original docs were even mixing br and p... So really, my PR is just a real bug fix, nothing else.

I agree that your PR definitely improves the situation. It's just something that came to my mind while reviewing this.

mkarg · 2018-04-28T14:23:59Z

@chkal Sorry for the confusion. Yes, the expected scenario is that I will merge #622 first if no -1 is voted within the next 7 days.

mkarg · 2018-05-05T22:41:52Z

@eclipsewebmaster I do not understand why ip validation fails. It says @ggam's signed-off-by is invalid, but in fact it looks pretty valid.

Signed-off-by: Markus KARG <markus@headcrashing.eu>

mkarg · 2018-05-08T11:05:07Z

@ggam You signed with Signed-off-by: Guillermo González de Agüero <z06.guillermo@gmail.com>. Is that the exact same email address you also registered with your Eclipse Foundation account and Github account? Otherwise this would explain why the ip-validation check still fails.

ggam · 2018-05-08T11:14:57Z

@mkarg yes that's the email on both sites. @waynebeaton any idea?

mkarg · 2018-05-08T11:21:56Z

@eclipsewebmaster @ggam See also https://bugs.eclipse.org/bugs/show_bug.cgi?id=534451.

Signed-off-by: Guillermo González de Agüero and Markus KARG <markus@headcrashing.eu>

mkarg · 2018-05-10T10:03:13Z

@ggam I did some checks and I need to say that the ip validation check works pretty well as soon as I replace z06.guillermo@gmail.com by my own email address. It even works if I write your full name with all umlauts and accents into the sign-off. So it MUST be the wrong address! Please do check once more that you really filed exactly that address with all your paperwork and that this really is the address linked with your ggam account on both, Github user account and Eclipse Foundation user account.

@eclipsewebmaster Can you please check the actual reason why the ip-validation of user ggam fails always? Maybe you can see the address actually expected for that user account?

Meanwhile I signed your commit with my address and added both user names. This is pretty legal as the ip check wants EITHER you or me to sign all commits. This seems to work so we can go on with this PR.

mkarg · 2018-05-10T10:10:55Z

@chkal Can you please redo your approval? This is needed as the recent fixing of signed-off-by problems with ggam's contribution invalidated your existing voting.

@jimma I would be more than happy if you would approve this PR. It is an extension to the PR #622 which you approved recently. Thanks. :-)

chkal · 2018-05-10T14:32:25Z

@mkarg Actually I didn't approve the changes yet, I just added the comments. I'll need some time to checkout your changes and verify the generated Javadocs. A few aspects look weird to me (before and after your changes) and I want to verify the resulting HTML just to check that everything is fine (like @NameBinding which doesn't contain paragraph tags at all). I'll come back to you ASAP.

chkal

Sorry for the delay. I finally found some time to check some changes by comparing the new javadocs with the old ones.

I think that almost all the fixes are fine. I'm just a bit unsure regarding all the  element changes. There is at least one case for which the rendered output now differs from the original (see my comment). And IMHO the original looks better. Unfortunately repairing all these  elements is quite difficult and even the original javadocs don't always render in a nice way.

However, I'll approve the PR now. IMO it is important to enable the doclint checks. We can further cleanup the HTML later on to improve the visual output.

chkal · 2018-05-19T07:46:29Z

jaxrs-api/src/main/java/javax/ws/rs/NameBinding.java

@@ -25,7 +25,7 @@
 /**
 * Meta-annotation used to create name binding annotations for filters
 * and interceptors.
- * <p>


After removing this , the upper sentence isn't in its own paragraph any more. I don't think that this is correct. The rendered page looks weird, because there is no spacing between the first paragraph and the source code any more.

I will merge now but ask some rookie volunteer to improve the layout of the HTML.

mkarg added help wanted Extra attention is needed good first issue Good for newcomers javadoc labels Apr 21, 2018

mkarg added this to the 2.2 milestone Apr 21, 2018

mkarg self-assigned this Apr 21, 2018

mkarg mentioned this pull request Apr 21, 2018

Enabling JDK 8 Doclint Checks #623

Closed

mkarg added the wip label Apr 21, 2018

mkarg force-pushed the mkarg-enabling-doclint branch from abf27f7 to 166629b Compare April 22, 2018 13:57

mkarg changed the title ~~WIP: Enabling JDK 8 Doclint Checks (#623)~~ Enabling JDK 8 Doclint Checks (#623) Apr 22, 2018

mkarg removed good first issue Good for newcomers help wanted Extra attention is needed labels Apr 22, 2018

mkarg requested review from chkal and asoldano April 22, 2018 14:03

mkarg removed the wip label Apr 22, 2018

chkal reviewed Apr 28, 2018

View reviewed changes

mkarg added 2 commits May 8, 2018 13:01

Enabling doclint

49d6af7

Signed-off-by: Markus KARG <markus@headcrashing.eu>

No more Doclint ERRORs

cc1ab6d

Signed-off-by: Markus KARG <markus@headcrashing.eu>

mkarg force-pushed the mkarg-enabling-doclint branch from 166629b to 518114a Compare May 8, 2018 11:02

mkarg force-pushed the mkarg-enabling-doclint branch 4 times, most recently from ea59695 to f61faf7 Compare May 10, 2018 09:31

mkarg force-pushed the mkarg-enabling-doclint branch 5 times, most recently from 6a90447 to 03ea20f Compare May 10, 2018 09:54

Fixed JavaDoc warnings

6bf19c7

Signed-off-by: Guillermo González de Agüero and Markus KARG <markus@headcrashing.eu>

mkarg force-pushed the mkarg-enabling-doclint branch from 03ea20f to 6bf19c7 Compare May 10, 2018 09:55

mkarg requested a review from jimma May 10, 2018 10:06

asoldano approved these changes May 16, 2018

View reviewed changes

spericas self-requested a review May 16, 2018 15:04

spericas approved these changes May 16, 2018

View reviewed changes

chkal approved these changes May 19, 2018

View reviewed changes

mkarg merged commit e9d0335 into jakartaee:master May 19, 2018

mkarg deleted the mkarg-enabling-doclint branch June 2, 2018 13:59

mkarg added the project label Dec 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enabling JDK 8 Doclint Checks (#623) #624

Enabling JDK 8 Doclint Checks (#623) #624

mkarg commented Apr 21, 2018 •

edited

ggam commented Apr 22, 2018

mkarg commented Apr 22, 2018

ggam commented Apr 22, 2018 via email

chkal left a comment

chkal Apr 28, 2018

mkarg Apr 28, 2018

chkal Apr 28, 2018

mkarg Apr 28, 2018

chkal Apr 29, 2018

mkarg commented Apr 28, 2018

mkarg commented May 5, 2018

mkarg commented May 8, 2018

ggam commented May 8, 2018

mkarg commented May 8, 2018

mkarg commented May 10, 2018

mkarg commented May 10, 2018

chkal commented May 10, 2018

chkal left a comment

chkal May 19, 2018

mkarg May 19, 2018

Enabling JDK 8 Doclint Checks (#623) #624

Enabling JDK 8 Doclint Checks (#623) #624

Conversation

mkarg commented Apr 21, 2018 • edited

ggam commented Apr 22, 2018

mkarg commented Apr 22, 2018

ggam commented Apr 22, 2018 via email

chkal left a comment

Choose a reason for hiding this comment

chkal Apr 28, 2018

Choose a reason for hiding this comment

mkarg Apr 28, 2018

Choose a reason for hiding this comment

chkal Apr 28, 2018

Choose a reason for hiding this comment

mkarg Apr 28, 2018

Choose a reason for hiding this comment

chkal Apr 29, 2018

Choose a reason for hiding this comment

mkarg commented Apr 28, 2018

mkarg commented May 5, 2018

mkarg commented May 8, 2018

ggam commented May 8, 2018

mkarg commented May 8, 2018

mkarg commented May 10, 2018

mkarg commented May 10, 2018

chkal commented May 10, 2018

chkal left a comment

Choose a reason for hiding this comment

chkal May 19, 2018

Choose a reason for hiding this comment

mkarg May 19, 2018

Choose a reason for hiding this comment

mkarg commented Apr 21, 2018 •

edited