You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Kirk Lund <kl...@pivotal.io> on 2016/02/01 18:26:38 UTC
Re: Broken javadocs
When I remove this line:
diff --git a/build.gradle b/build.gradle
index 13afa17..c2c5e40 100755
--- a/build.gradle
+++ b/build.gradle
@@ -312,7 +312,6 @@ subprojects {
javadoc.classpath += configurations.provided
javadoc {
- options.addStringOption('Xdoclint:none', '-quiet')
options.encoding='UTF-8'
}
The result is a lot more javadoc warnings including missing @ tags
self-closing html elements. This seems to be overly restrictive and would
require a LOT more work than the 100s of broken tags that I already fixed
on feature/GEODE-805:
C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:226:
warning: no @param for ignoreMissingKey
public ResponseEntity<?> read(
^
C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:49:
error: self-closing element not allowed
* <p/>
^
C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\QueryAccessController.java:81:
error: self-closing element not allowed
* <p/>
Is there a build.gradle change that would turn the warnings on develop into
errors without increasing the restrictions even further?
Thanks,
Kirk
On Thu, Jan 28, 2016 at 12:54 PM, Kirk Lund <ki...@gmail.com> wrote:
> Thanks Nitin! I'm going to go ahead and fix all of the warnings and
> re-enable strict checking.
>
> If anyone else has already started this, please let me know.
>
> -Kirk
>
>
> On Thu, Jan 28, 2016 at 11:10 AM, Nitin Lamba <ni...@ampool.io> wrote:
>
> > Yes, there is.
> >
> > To re-enable strict javadoc checking in JDK8, you can remove this command
> > line option present in build.gradle today:
> >
> > javadoc {
> > options.addStringOption('Xdoclint:none', '-quiet')
> > }
> >
> > Once it is removed, the build will fail. Last checked, it was generating
> > more than 100 errors!
> >
> > Nitin
> >
> > ________________________________________
> > From: Kirk Lund <kl...@pivotal.io>
> > Sent: Thursday, January 28, 2016 10:58 AM
> > To: dev@geode.incubator.apache.org
> > Subject: Broken javadocs
> >
> > The build reports lots of broken javadocs. After fixing them, is there a
> > way (in gradle) to turn the warnings into errors that fail the build? I
> > would hate to go to all the effort of fixing these warnings and then see
> > people checkin more broken javadocs after that.
> >
> > Thanks,
> > Kirk
> >
>
Re: Broken javadocs
Posted by Anthony Baker <ab...@pivotal.io>.
The Java8 javadoc tool went crazy on strict compliance. The doc page shows that we could disable certain categories (e.g. missing) instead of all checks.
http://docs.oracle.com/javase/8/docs/technotes/tools/windows/javadoc.html
Anthony
> On Feb 1, 2016, at 11:27 AM, Kirk Lund <kl...@pivotal.io> wrote:
>
> It's not really an issue of internal vs User API...
>
> I just merged feature/GEODE-805 into develop. The build is now currently
> free of any and all javadoc warnings.
>
> Here's the problem: If someone now commits a class with a malformed
> javadoc, the build output will start to collect new javadoc warnings rather
> than causing the build to fail which would thus prevent the person from
> committing. The old Ant build failed if any new javadoc problems were
> introduced and this prevented the situation we are in (or were just in).
>
> Removing the line from the javadocs block, however, will force every class
> to define every javadoc tag and that is undesirable. I only want to block
> developers from committing changes with new malformed javadoc warnings that
> we'll have to fix in a similar effort again.
>
> I ca understand the desire to only generate javadocs for classes NOT in
> internal packages or in the test java src trees. However, I believe that
> removing the line in question would still require that we fix more javadocs
> than I just fixed. This would also allow us to accumulate lots of new
> broken javadocs on the internal classes and those javadocs are very useful
> to new and old contributors working on the geode code base.
>
> -Kirk
>
>
> On Mon, Feb 1, 2016 at 10:33 AM, Anilkumar Gingade <ag...@pivotal.io>
> wrote:
>
>> Agree With Dan...
>>
>> -Anil.
>>
>>
>> On Mon, Feb 1, 2016 at 10:20 AM, Dan Smith <ds...@pivotal.io> wrote:
>>
>>> For GEODE-518, I think we should just generate javadocs for the public
>> API
>>> and not worry about the internal classes right now. That might make this
>> a
>>> lot easier if we do that fix.
>>>
>>> -Dan
>>>
>>> On Mon, Feb 1, 2016 at 9:26 AM, Kirk Lund <kl...@pivotal.io> wrote:
>>>
>>>> When I remove this line:
>>>>
>>>> diff --git a/build.gradle b/build.gradle
>>>> index 13afa17..c2c5e40 100755
>>>> --- a/build.gradle
>>>> +++ b/build.gradle
>>>> @@ -312,7 +312,6 @@ subprojects {
>>>> javadoc.classpath += configurations.provided
>>>>
>>>> javadoc {
>>>> - options.addStringOption('Xdoclint:none', '-quiet')
>>>> options.encoding='UTF-8'
>>>> }
>>>>
>>>> The result is a lot more javadoc warnings including missing @ tags
>>>> self-closing html elements. This seems to be overly restrictive and
>> would
>>>> require a LOT more work than the 100s of broken tags that I already
>> fixed
>>>> on feature/GEODE-805:
>>>>
>>>>
>>>>
>>>
>> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:226:
>>>> warning: no @param for ignoreMissingKey
>>>> public ResponseEntity<?> read(
>>>> ^
>>>>
>>>>
>>>
>> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:49:
>>>> error: self-closing element not allowed
>>>> * <p/>
>>>> ^
>>>>
>>>>
>>>
>> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\QueryAccessController.java:81:
>>>> error: self-closing element not allowed
>>>> * <p/>
>>>>
>>>> Is there a build.gradle change that would turn the warnings on develop
>>> into
>>>> errors without increasing the restrictions even further?
>>>>
>>>> Thanks,
>>>> Kirk
>>>>
>>>>
>>>> On Thu, Jan 28, 2016 at 12:54 PM, Kirk Lund <ki...@gmail.com>
>> wrote:
>>>>
>>>>> Thanks Nitin! I'm going to go ahead and fix all of the warnings and
>>>>> re-enable strict checking.
>>>>>
>>>>> If anyone else has already started this, please let me know.
>>>>>
>>>>> -Kirk
>>>>>
>>>>>
>>>>> On Thu, Jan 28, 2016 at 11:10 AM, Nitin Lamba <ni...@ampool.io>
>> wrote:
>>>>>
>>>>>> Yes, there is.
>>>>>>
>>>>>> To re-enable strict javadoc checking in JDK8, you can remove this
>>>> command
>>>>>> line option present in build.gradle today:
>>>>>>
>>>>>> javadoc {
>>>>>> options.addStringOption('Xdoclint:none', '-quiet')
>>>>>> }
>>>>>>
>>>>>> Once it is removed, the build will fail. Last checked, it was
>>>> generating
>>>>>> more than 100 errors!
>>>>>>
>>>>>> Nitin
>>>>>>
>>>>>> ________________________________________
>>>>>> From: Kirk Lund <kl...@pivotal.io>
>>>>>> Sent: Thursday, January 28, 2016 10:58 AM
>>>>>> To: dev@geode.incubator.apache.org
>>>>>> Subject: Broken javadocs
>>>>>>
>>>>>> The build reports lots of broken javadocs. After fixing them, is
>>> there
>>>> a
>>>>>> way (in gradle) to turn the warnings into errors that fail the
>>> build? I
>>>>>> would hate to go to all the effort of fixing these warnings and
>> then
>>>> see
>>>>>> people checkin more broken javadocs after that.
>>>>>>
>>>>>> Thanks,
>>>>>> Kirk
>>>>>>
>>>>>
>>>>
>>>
>>
Re: Broken javadocs
Posted by Kirk Lund <kl...@pivotal.io>.
It's not really an issue of internal vs User API...
I just merged feature/GEODE-805 into develop. The build is now currently
free of any and all javadoc warnings.
Here's the problem: If someone now commits a class with a malformed
javadoc, the build output will start to collect new javadoc warnings rather
than causing the build to fail which would thus prevent the person from
committing. The old Ant build failed if any new javadoc problems were
introduced and this prevented the situation we are in (or were just in).
Removing the line from the javadocs block, however, will force every class
to define every javadoc tag and that is undesirable. I only want to block
developers from committing changes with new malformed javadoc warnings that
we'll have to fix in a similar effort again.
I ca understand the desire to only generate javadocs for classes NOT in
internal packages or in the test java src trees. However, I believe that
removing the line in question would still require that we fix more javadocs
than I just fixed. This would also allow us to accumulate lots of new
broken javadocs on the internal classes and those javadocs are very useful
to new and old contributors working on the geode code base.
-Kirk
On Mon, Feb 1, 2016 at 10:33 AM, Anilkumar Gingade <ag...@pivotal.io>
wrote:
> Agree With Dan...
>
> -Anil.
>
>
> On Mon, Feb 1, 2016 at 10:20 AM, Dan Smith <ds...@pivotal.io> wrote:
>
> > For GEODE-518, I think we should just generate javadocs for the public
> API
> > and not worry about the internal classes right now. That might make this
> a
> > lot easier if we do that fix.
> >
> > -Dan
> >
> > On Mon, Feb 1, 2016 at 9:26 AM, Kirk Lund <kl...@pivotal.io> wrote:
> >
> > > When I remove this line:
> > >
> > > diff --git a/build.gradle b/build.gradle
> > > index 13afa17..c2c5e40 100755
> > > --- a/build.gradle
> > > +++ b/build.gradle
> > > @@ -312,7 +312,6 @@ subprojects {
> > > javadoc.classpath += configurations.provided
> > >
> > > javadoc {
> > > - options.addStringOption('Xdoclint:none', '-quiet')
> > > options.encoding='UTF-8'
> > > }
> > >
> > > The result is a lot more javadoc warnings including missing @ tags
> > > self-closing html elements. This seems to be overly restrictive and
> would
> > > require a LOT more work than the 100s of broken tags that I already
> fixed
> > > on feature/GEODE-805:
> > >
> > >
> > >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:226:
> > > warning: no @param for ignoreMissingKey
> > > public ResponseEntity<?> read(
> > > ^
> > >
> > >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:49:
> > > error: self-closing element not allowed
> > > * <p/>
> > > ^
> > >
> > >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\QueryAccessController.java:81:
> > > error: self-closing element not allowed
> > > * <p/>
> > >
> > > Is there a build.gradle change that would turn the warnings on develop
> > into
> > > errors without increasing the restrictions even further?
> > >
> > > Thanks,
> > > Kirk
> > >
> > >
> > > On Thu, Jan 28, 2016 at 12:54 PM, Kirk Lund <ki...@gmail.com>
> wrote:
> > >
> > > > Thanks Nitin! I'm going to go ahead and fix all of the warnings and
> > > > re-enable strict checking.
> > > >
> > > > If anyone else has already started this, please let me know.
> > > >
> > > > -Kirk
> > > >
> > > >
> > > > On Thu, Jan 28, 2016 at 11:10 AM, Nitin Lamba <ni...@ampool.io>
> wrote:
> > > >
> > > > > Yes, there is.
> > > > >
> > > > > To re-enable strict javadoc checking in JDK8, you can remove this
> > > command
> > > > > line option present in build.gradle today:
> > > > >
> > > > > javadoc {
> > > > > options.addStringOption('Xdoclint:none', '-quiet')
> > > > > }
> > > > >
> > > > > Once it is removed, the build will fail. Last checked, it was
> > > generating
> > > > > more than 100 errors!
> > > > >
> > > > > Nitin
> > > > >
> > > > > ________________________________________
> > > > > From: Kirk Lund <kl...@pivotal.io>
> > > > > Sent: Thursday, January 28, 2016 10:58 AM
> > > > > To: dev@geode.incubator.apache.org
> > > > > Subject: Broken javadocs
> > > > >
> > > > > The build reports lots of broken javadocs. After fixing them, is
> > there
> > > a
> > > > > way (in gradle) to turn the warnings into errors that fail the
> > build? I
> > > > > would hate to go to all the effort of fixing these warnings and
> then
> > > see
> > > > > people checkin more broken javadocs after that.
> > > > >
> > > > > Thanks,
> > > > > Kirk
> > > > >
> > > >
> > >
> >
>
Re: Broken javadocs
Posted by Anilkumar Gingade <ag...@pivotal.io>.
Agree With Dan...
-Anil.
On Mon, Feb 1, 2016 at 10:20 AM, Dan Smith <ds...@pivotal.io> wrote:
> For GEODE-518, I think we should just generate javadocs for the public API
> and not worry about the internal classes right now. That might make this a
> lot easier if we do that fix.
>
> -Dan
>
> On Mon, Feb 1, 2016 at 9:26 AM, Kirk Lund <kl...@pivotal.io> wrote:
>
> > When I remove this line:
> >
> > diff --git a/build.gradle b/build.gradle
> > index 13afa17..c2c5e40 100755
> > --- a/build.gradle
> > +++ b/build.gradle
> > @@ -312,7 +312,6 @@ subprojects {
> > javadoc.classpath += configurations.provided
> >
> > javadoc {
> > - options.addStringOption('Xdoclint:none', '-quiet')
> > options.encoding='UTF-8'
> > }
> >
> > The result is a lot more javadoc warnings including missing @ tags
> > self-closing html elements. This seems to be overly restrictive and would
> > require a LOT more work than the 100s of broken tags that I already fixed
> > on feature/GEODE-805:
> >
> >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:226:
> > warning: no @param for ignoreMissingKey
> > public ResponseEntity<?> read(
> > ^
> >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:49:
> > error: self-closing element not allowed
> > * <p/>
> > ^
> >
> >
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\QueryAccessController.java:81:
> > error: self-closing element not allowed
> > * <p/>
> >
> > Is there a build.gradle change that would turn the warnings on develop
> into
> > errors without increasing the restrictions even further?
> >
> > Thanks,
> > Kirk
> >
> >
> > On Thu, Jan 28, 2016 at 12:54 PM, Kirk Lund <ki...@gmail.com> wrote:
> >
> > > Thanks Nitin! I'm going to go ahead and fix all of the warnings and
> > > re-enable strict checking.
> > >
> > > If anyone else has already started this, please let me know.
> > >
> > > -Kirk
> > >
> > >
> > > On Thu, Jan 28, 2016 at 11:10 AM, Nitin Lamba <ni...@ampool.io> wrote:
> > >
> > > > Yes, there is.
> > > >
> > > > To re-enable strict javadoc checking in JDK8, you can remove this
> > command
> > > > line option present in build.gradle today:
> > > >
> > > > javadoc {
> > > > options.addStringOption('Xdoclint:none', '-quiet')
> > > > }
> > > >
> > > > Once it is removed, the build will fail. Last checked, it was
> > generating
> > > > more than 100 errors!
> > > >
> > > > Nitin
> > > >
> > > > ________________________________________
> > > > From: Kirk Lund <kl...@pivotal.io>
> > > > Sent: Thursday, January 28, 2016 10:58 AM
> > > > To: dev@geode.incubator.apache.org
> > > > Subject: Broken javadocs
> > > >
> > > > The build reports lots of broken javadocs. After fixing them, is
> there
> > a
> > > > way (in gradle) to turn the warnings into errors that fail the
> build? I
> > > > would hate to go to all the effort of fixing these warnings and then
> > see
> > > > people checkin more broken javadocs after that.
> > > >
> > > > Thanks,
> > > > Kirk
> > > >
> > >
> >
>
Re: Broken javadocs
Posted by Dan Smith <ds...@pivotal.io>.
For GEODE-518, I think we should just generate javadocs for the public API
and not worry about the internal classes right now. That might make this a
lot easier if we do that fix.
-Dan
On Mon, Feb 1, 2016 at 9:26 AM, Kirk Lund <kl...@pivotal.io> wrote:
> When I remove this line:
>
> diff --git a/build.gradle b/build.gradle
> index 13afa17..c2c5e40 100755
> --- a/build.gradle
> +++ b/build.gradle
> @@ -312,7 +312,6 @@ subprojects {
> javadoc.classpath += configurations.provided
>
> javadoc {
> - options.addStringOption('Xdoclint:none', '-quiet')
> options.encoding='UTF-8'
> }
>
> The result is a lot more javadoc warnings including missing @ tags
> self-closing html elements. This seems to be overly restrictive and would
> require a LOT more work than the 100s of broken tags that I already fixed
> on feature/GEODE-805:
>
>
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:226:
> warning: no @param for ignoreMissingKey
> public ResponseEntity<?> read(
> ^
>
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\PdxBasedCrudController.java:49:
> error: self-closing element not allowed
> * <p/>
> ^
>
> C:\dev\geode\gemfire-web-api\src\main\java\com\gemstone\gemfire\rest\internal\web\controllers\QueryAccessController.java:81:
> error: self-closing element not allowed
> * <p/>
>
> Is there a build.gradle change that would turn the warnings on develop into
> errors without increasing the restrictions even further?
>
> Thanks,
> Kirk
>
>
> On Thu, Jan 28, 2016 at 12:54 PM, Kirk Lund <ki...@gmail.com> wrote:
>
> > Thanks Nitin! I'm going to go ahead and fix all of the warnings and
> > re-enable strict checking.
> >
> > If anyone else has already started this, please let me know.
> >
> > -Kirk
> >
> >
> > On Thu, Jan 28, 2016 at 11:10 AM, Nitin Lamba <ni...@ampool.io> wrote:
> >
> > > Yes, there is.
> > >
> > > To re-enable strict javadoc checking in JDK8, you can remove this
> command
> > > line option present in build.gradle today:
> > >
> > > javadoc {
> > > options.addStringOption('Xdoclint:none', '-quiet')
> > > }
> > >
> > > Once it is removed, the build will fail. Last checked, it was
> generating
> > > more than 100 errors!
> > >
> > > Nitin
> > >
> > > ________________________________________
> > > From: Kirk Lund <kl...@pivotal.io>
> > > Sent: Thursday, January 28, 2016 10:58 AM
> > > To: dev@geode.incubator.apache.org
> > > Subject: Broken javadocs
> > >
> > > The build reports lots of broken javadocs. After fixing them, is there
> a
> > > way (in gradle) to turn the warnings into errors that fail the build? I
> > > would hate to go to all the effort of fixing these warnings and then
> see
> > > people checkin more broken javadocs after that.
> > >
> > > Thanks,
> > > Kirk
> > >
> >
>