Re: [PATCH] Add TREAT_WARNINGS_AS_ERROR option
Albert <[email protected]> Wed, 9 Dec 2015 19:33:08 +0100
| Newsgroups | gmane.text.doxygen.devel |
|---|---|
| Message-ID | <CABOOx7YC=vjtdu2c5PnMVipeaNUJJfJfGLwYVZ-F1TpCWBiWJA@mail.gmail.com> |
--===============4112308624336396442== Content-Type: multipart/alternative; boundary=001a114a847c63a79805267b52a4 --001a114a847c63a79805267b52a4 Content-Type: text/plain; charset=UTF-8 Hi Gautier, Thanks for you responses to my suggestions. 1) continuation on errors. In principle I think you are right, though the word error has a higher "visibility" than warning. An example where an error is given but the process continues is when the image from the PROJECT_LOGO does not exist. This significantly influences the output. So maybe either we should have some more categories: - fatal same as current error ans stop generation (will break some builds as some people , I do it myself, search for the word 'error' at the beginning of the sentence - error same as current error, but process is continuing - warning as it is now 2) extra exit code for warnings I see you objection that it possibly will break, from the compatibility point of view is probably a less good idea 3) Proposed patch does stop at first warning, my idea was more replacing the word "warning" by "error" for higher "visibility". 4) giving warnings and errors a number we agree on this 5) inspecting the warnings.log file I think in any case one uses a build system to build the code and in one way or another the build errors will be reported as well., possibly by an external tool as well. Checking the warnings.log file would only be a test whether or not the warnings file is empty or not, displaying the warnings and abort the build process. So I don't see this as a big problem all build systems will support something like this. Albert On Tue, Dec 8, 2015 at 10:35 AM, Gautier Pelloux-Prayer <[email protected]> wrote: > Hi Albert, > > Thanks for your detailled opinion. Please see my inlined remarks below. > > Gautier > > > On 06 Dec 2015, at 12:03, Albert <[email protected]> wrote: > > > > In principle all the errors and warnings doxygen gives should be removed > by the engineers. > > > > Doxygen distinguish between errors and warnings, where errors possibly > corrupt the documentation building or are serious enough, in the eyes of > the doxygen developers to stop the generation of the documentation. In case > of an error the doxygen developer can also choose to give an error message > but to continue anyway. > Should not it be a warning then? An error should have a consistent > behavior during all processing, should it not? > > > > When we would have the possibility to promote all the warnings to errors > (and we should also stop at all errors that currently don't stop), it would > mean that, when using the option, after each error a rerun is necessary. I > think this is a bit overdone. > Well, that's the current behavior of -Werror for compilers. Indeed on > first execution you will have a lot of stop&retry, but once it is done, you > should not have many errors. And on first execution, you can temporary use > -Wno-error to see all errors immediately if needed. > > I see some possibilities: > > - in case doxygen discovers warnings it does not finish with exit code 0 > but with e.g. exit code 2 (and use exit code 1 for the errors on which it > now already stops). > This solution suits me but if that possibility is adopted it might break > many build systems where doxygen is expected to exit successfully in any > case. Would it be a problem? > > - when using the proposed option the message is given as an error, but > the process does not stop. I don't see this as an improvement as one can > now catch the warnings in a log file and check them. > I am not sure to understand what do you mean there - proposed patch should > exit on first warning? > > - all warnings get a number and the user can "promote" in the Doxyfile > some warnings to, fatal, errors. This would be quite a big effort as all > the messages have to be categorized (I've been thinking about this some > time ago, but have not started to implement this as it is a lot of work and > does not bring enough in my opinion, a quick estimation 700 messages to be > checked). > Well, I agree with you that it would be both an ideal solution and a big > effort which does not bring enough ;-). > > > > The procedure I follow more or less is: Each time I'm building > (overnight builds) the software I catch the warnings and errors in a > warnings.log file. When this warnings.log file contains a message (or a new > message compared to the last time the build was done) an email is send to > the engineer who, last. changed the source file. > That's the step I want to avoid: having an external script/tool checking > doxygen output to see if anything went wrong, while doxygen could tell me > directly if I broke something. However this is more or less what I am > aiming for. > > > > > > Albert > > > > On Thu, Dec 3, 2015 at 9:34 PM, Gautier <[email protected]> wrote: > > Hi list; > > > > As stated on Github[1], I would like to add an option behaving similarly > > to compilers' -Werror option: any warning generated during Doxygen > > execution will abort it immediately. > > > > This option is, in my opinion, valuable to keep documentation up-to-date > > with code changes, specially when multiple people are working altogether. > > I will not blame anyone to not think all consequence of a code change, > > but scripts can help him/her from breaking anything: > > > > * strict compiler options (-Werror, -Wall, etc.) prevents at least some > > quick&dirty code and obvious bugs. Compilers will warn beginners about > > code which will not work (out of bounds errors, etc.), but most of the > > time it's simply about a typo / useless dead code. > > * Git hook preventing users from pushing invalid commits [2]. We are > > intensively using git submodules for instance - it's quite easy to push > > an invalid submodule reference to git: > > * if you forgot to run "git submodule update --recursive" after > > pulling remote changes following by a "git commit -a", leading to > > unwilling submodule downgrading. > > * if you're referencing a new submodule revision... which you > > forgot to push first! > > * To have better code consistency, following a single code style using > > clang-format[3], similarly to what systemd does[4]. > > > > I think that all of us already encountered one or several of these > > issues at least once ;-). Having tools checking these errors > > automatically allow me not to worry about it anymore. > > Our documentation contains nowadays many errors simply because Doxygen > > let us do that and we did not look at """inoffensive""" warnings, since > > they are not preventing us from continuing - they are lost in the > > verbosity of build logs. > > > > I am the only one which would like such an option, and if so: how do you > > keep your documentation up-to-date? > > > > Cheers, > > > > Gautier > > > > [1] https://github.com/doxygen/doxygen/pull/412 > > [2] https://gist.github.com/bagage/bdca3d4b66d43db7a5e3 > > [3] http://clang.llvm.org/docs/ClangFormat.html > > [4] https://github.com/systemd/systemd/blob/master/autogen.sh > > > > > ------------------------------------------------------------------------------ > > Go from Idea to Many App Stores Faster with Intel(R) XDK > > Give your users amazing mobile app experiences with Intel(R) XDK. > > Use one codebase in this all-in-one HTML5 development environment. > > Design, debug & build mobile apps & 2D/3D high-impact games for multiple > OSs. > > http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140 > > _______________________________________________ > > Doxygen-develop mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/doxygen-develop > > > > --001a114a847c63a79805267b52a4 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Hi Gautier,<br><br>Thanks for you responses to my suggesti= ons.<br>1) continuation on errors.<br>=C2=A0=C2=A0 In principle I think you= are right, though the word error has a higher "visibility" than = warning. An example where an error is given but the process continues is wh= en the image from the PROJECT_LOGO does not exist. This significantly influ= ences the output.<br>=C2=A0=C2=A0 So maybe either we should have some more = categories:<br>=C2=A0=C2=A0 - fatal same as current error ans stop generati= on (will break some builds as some people , I do it myself, search for the = word 'error' at the beginning of the sentence<br>=C2=A0=C2=A0 - err= or same as current error, but process is continuing<br>=C2=A0=C2=A0 - warni= ng as it is now<br>2) extra exit code for warnings<br>=C2=A0=C2=A0 I see yo= u objection that it possibly will break, from the compatibility point of vi= ew is probably a less good idea<br>3) Proposed patch does stop at first war= ning, my idea was more replacing the word "warning" by "erro= r" for higher "visibility".<br>4) giving warnings and errors= a number<br>=C2=A0=C2=A0 we agree on this<br>5) inspecting the warnings.lo= g file<br>=C2=A0=C2=A0 I think in any case one uses a build system to build= the code and in one way or another the build errors will be reported as we= ll., possibly by an external tool as well.<br>=C2=A0=C2=A0 Checking the war= nings.log file would only be a test whether or not the warnings file is emp= ty or not, displaying the warnings and abort the build process. So I don= 9;t see this as a big problem all build systems will support something like= this.<br><br>Albert<br><br><br></div><div class=3D"gmail_extra"><br><div c= lass=3D"gmail_quote">On Tue, Dec 8, 2015 at 10:35 AM, Gautier Pelloux-Praye= r <span dir=3D"ltr"><<a href=3D"mailto:[email protected]" target=3D"_bla= nk">[email protected]</a>></span> wrote:<br><blockquote class=3D"gmail_q= uote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1e= x">Hi Albert,<br> <br> Thanks for your detailled opinion. Please see my inlined remarks below.<br> <br> Gautier<br> <span class=3D""><br> > On 06 Dec 2015, at 12:03, Albert <<a href=3D"mailto:albert.tests@gm= ail.com">[email protected]</a>> wrote:<br> ><br> > In principle all the errors and warnings doxygen gives should be remov= ed by the engineers.<br> ><br> > Doxygen distinguish between errors and warnings, where errors possibly= corrupt the documentation building or are serious enough, in the eyes of t= he doxygen developers to stop the generation of the documentation. In case = of an error the doxygen developer can also choose to give an error message = but to continue anyway.<br> </span>Should not it be a warning then? An error should have a consistent b= ehavior during all processing, should it not?<br> <span class=3D"">><br> > When we would have the possibility to promote all the warnings to erro= rs (and we should also stop at all errors that currently don't stop), i= t would mean that, when using the option, after each error a rerun is neces= sary. I think this is a bit overdone.<br> </span>Well, that's the current behavior of -Werror for compilers. Inde= ed on first execution you will have a lot of stop&retry, but once it is= done, you should not have many errors. And on first execution, you can tem= porary use -Wno-error to see all errors immediately if needed.<br> <span class=3D"">> I see some possibilities:<br> > - in case doxygen discovers warnings it does not finish with exit code= 0 but with e.g. exit code 2 (and use exit code 1 for the errors on which i= t now already stops).<br> </span>This solution suits me but if that possibility is adopted it might b= reak many build systems where doxygen is expected to exit successfully in a= ny case. Would it be a problem?<br> <span class=3D"">> - when using the proposed option the message is given= as an error, but the process does not stop. I don't see this as an imp= rovement as one can now catch the warnings in a log file and check them.<br= > </span>I am not sure to understand what do you mean there - proposed patch = should exit on first warning?<br> <span class=3D"">> - all warnings get a number and the user can "pr= omote" in the Doxyfile some warnings to, fatal, errors. This would be = quite a big effort as all the messages have to be categorized (I've bee= n thinking about this some time ago, but have not started to implement this= as it is a lot of work and does not bring enough in my opinion, a quick es= timation 700 messages to be checked).<br> </span>Well, I agree with you that it would be both an ideal solution and a= big effort which does not bring enough ;-).<br> <span class=3D"">><br> > The procedure I follow more or less is: Each time I'm building (ov= ernight builds) the software I catch the warnings and errors in a warnings.= log file. When this warnings.log file contains a message (or a new message = compared to the last time the build was done) an email is send to the engin= eer who, last. changed the source file.<br> </span>That's the step I want to avoid: having an external script/tool = checking doxygen output to see if anything went wrong, while doxygen could = tell me directly if I broke something. However this is more or less what I = am aiming for.<br> <div class=3D"HOEnZb"><div class=3D"h5">><br> ><br> > Albert<br> ><br> > On Thu, Dec 3, 2015 at 9:34 PM, Gautier <<a href=3D"mailto:gautier@= damsy.net">[email protected]</a>> wrote:<br> > Hi list;<br> ><br> > As stated on Github[1], I would like to add an option behaving similar= ly<br> > to compilers' -Werror option: any warning generated during Doxygen= <br> > execution will abort it immediately.<br> ><br> > This option is, in my opinion, valuable to keep documentation up-to-da= te<br> > with code changes, specially when multiple people are working altogeth= er.<br> > I will not blame anyone to not think all consequence of a code change,= <br> > but scripts can help him/her from breaking anything:<br> ><br> > * strict compiler options (-Werror, -Wall, etc.) prevents at least som= e<br> > quick&dirty code and obvious bugs. Compilers will warn beginners a= bout<br> > code which will not work (out of bounds errors, etc.), but most of the= <br> > time it's simply about a typo / useless dead code.<br> > * Git hook preventing users from pushing invalid commits [2]. We are<b= r> > intensively using git submodules for instance - it's quite easy to= push<br> > an invalid submodule reference to git:<br> >=C2=A0 =C2=A0 =C2=A0 * if you forgot to run "git submodule update = --recursive" after<br> > pulling remote changes following by a "git commit -a", leadi= ng to<br> > unwilling submodule downgrading.<br> >=C2=A0 =C2=A0 =C2=A0 * if you're referencing a new submodule revisi= on... which you<br> > forgot to push first!<br> > * To have better code consistency, following a single code style using= <br> > clang-format[3], similarly to what systemd does[4].<br> ><br> > I think that all of us already encountered one or several of these<br> > issues at least once ;-). Having tools checking these errors<br> > automatically allow me not to worry about it anymore.<br> > Our documentation contains nowadays many errors simply because Doxygen= <br> > let us do that and we did not look at """inoffensive&qu= ot;"" warnings, since<br> > they are not preventing us from continuing - they are lost in the<br> > verbosity of build logs.<br> ><br> > I am the only one which would like such an option, and if so: how do y= ou<br> > keep your documentation up-to-date?<br> ><br> > Cheers,<br> ><br> > Gautier<br> ><br> > [1] <a href=3D"https://github.com/doxygen/doxygen/pull/412" rel=3D"nor= eferrer" target=3D"_blank">https://github.com/doxygen/doxygen/pull/412</a><= br> > [2] <a href=3D"https://gist.github.com/bagage/bdca3d4b66d43db7a5e3" re= l=3D"noreferrer" target=3D"_blank">https://gist.github.com/bagage/bdca3d4b6= 6d43db7a5e3</a><br> > [3] <a href=3D"http://clang.llvm.org/docs/ClangFormat.html" rel=3D"nor= eferrer" target=3D"_blank">http://clang.llvm.org/docs/ClangFormat.html</a><= br> > [4] <a href=3D"https://github.com/systemd/systemd/blob/master/autogen.= sh" rel=3D"noreferrer" target=3D"_blank">https://github.com/systemd/systemd= /blob/master/autogen.sh</a><br> ><br> > ----------------------------------------------------------------------= --------<br> > Go from Idea to Many App Stores Faster with Intel(R) XDK<br> > Give your users amazing mobile app experiences with Intel(R) XDK.<br> > Use one codebase in this all-in-one HTML5 development environment.<br> > Design, debug & build mobile apps & 2D/3D high-impact games fo= r multiple OSs.<br> > <a href=3D"http://pubads.g.doubleclick.net/gampad/clk?id=3D254741911&a= mp;iu=3D/4140" rel=3D"noreferrer" target=3D"_blank">http://pubads.g.doublec= lick.net/gampad/clk?id=3D254741911&iu=3D/4140</a><br> > _______________________________________________<br> > Doxygen-develop mailing list<br> > <a href=3D"mailto:[email protected]">Doxygen-devel= [email protected]</a><br> > <a href=3D"https://lists.sourceforge.net/lists/listinfo/doxygen-develo= p" rel=3D"noreferrer" target=3D"_blank">https://lists.sourceforge.net/lists= /listinfo/doxygen-develop</a><br> ><br> <br> </div></div></blockquote></div><br></div> --001a114a847c63a79805267b52a4-- --===============4112308624336396442== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ --===============4112308624336396442== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Doxygen-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/doxygen-develop --===============4112308624336396442==--