Move o.a.f.complexscripts.bidi.BidiClass to build/gensrc

[Vincent Hennebert <vh...@gmail.com>]

Hi,

I’ve recently stumbled upon this o.a.f.complexscripts.bidi.BidiClass
that appears to be generated. I think it should be moved to the
build/gensrc directory. Reasons are:
• It doesn’t need to be understood and maintained by developers in the
  same way as other classes. Changes will be made to the generator and
  not this class. Therefore it doesn’t need to follow the usual style
  and readability rules.
• Despite the warning at the beginning of the class, there is still
  a risk that it is accidentally modified during some refactoring
  session.
• There is also a risk that the generator is modified and this class not
  re-generated, introducing a discrepancy that may take some time to be
  tracked down.

Like any other generated file, I think the generation should happen at
any clean build.

Any objection?

Thanks,
Vincent

Re: Move o.a.f.complexscripts.bidi.BidiClass to build/gensrc

[Glenn Adams <gl...@skynav.com>]

On Fri, Nov 23, 2012 at 11:48 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> On 22/11/12 17:08, Glenn Adams wrote:
> > What do you mean by "moved to build/gensrc"? There is no build/gensrc
> > directory in the FOP source tree.
> >
> > Are you suggesting it be regenerated at build time? If so, then that
> would
> > slow the build process unnecessarily since it requires downloading data
> > from the unicode web site. Further, there is no logical need to
> regenerate
> > it at build time any more than there is a need to rebuild
> > LineBreakUtils.java.
>
> Well, then maybe we can move it to a new src/generated directory or
> something like that? I think it would be safer to separate generated
> classes from the ones that are manually edited.
>

How about expanding the fileset definition below to make use of a
patternset definition that excludes the specific generated files?

   <checkstyle config="${checkstyle.config}" failonviolation="false">
      <classpath>
        <path refid="checkstyle-classpath"/>
        <pathelement location="${build.classes.dir}"/>
        <pathelement location="${build.sandbox-classes.dir}"/>
        <pathelement location="${build.codegen-classes.dir}"/>
      </classpath>
*      <fileset dir="${src.dir}" includes="**/*.java"/>*
      <formatter type="xml" toFile="${build.dir}/report_checkstyle.xml"/>
    </checkstyle>

Re: Move o.a.f.complexscripts.bidi.BidiClass to build/gensrc

[Vincent Hennebert <vh...@gmail.com>]

On 22/11/12 17:08, Glenn Adams wrote:
> What do you mean by "moved to build/gensrc"? There is no build/gensrc
> directory in the FOP source tree.
> 
> Are you suggesting it be regenerated at build time? If so, then that would
> slow the build process unnecessarily since it requires downloading data
> from the unicode web site. Further, there is no logical need to regenerate
> it at build time any more than there is a need to rebuild
> LineBreakUtils.java.

Well, then maybe we can move it to a new src/generated directory or
something like that? I think it would be safer to separate generated
classes from the ones that are manually edited.


> None of the rationale you provide below are compelling. So I object to this
> proposed change.
> 
> On Thu, Nov 22, 2012 at 2:52 AM, Vincent Hennebert <vh...@gmail.com>wrote:
> 
>> Hi,
>>
>> I’ve recently stumbled upon this o.a.f.complexscripts.bidi.BidiClass
>> that appears to be generated. I think it should be moved to the
>> build/gensrc directory. Reasons are:
>> • It doesn’t need to be understood and maintained by developers in the
>>   same way as other classes. Changes will be made to the generator and
>>   not this class. Therefore it doesn’t need to follow the usual style
>>   and readability rules.
>> • Despite the warning at the beginning of the class, there is still
>>   a risk that it is accidentally modified during some refactoring
>>   session.
>> • There is also a risk that the generator is modified and this class not
>>   re-generated, introducing a discrepancy that may take some time to be
>>   tracked down.
>>
>> Like any other generated file, I think the generation should happen at
>> any clean build.
>>
>> Any objection?
>>
>> Thanks,
>> Vincent
>>
> 

Re: Move o.a.f.complexscripts.bidi.BidiClass to build/gensrc

[Glenn Adams <gl...@skynav.com>]

What do you mean by "moved to build/gensrc"? There is no build/gensrc
directory in the FOP source tree.

Are you suggesting it be regenerated at build time? If so, then that would
slow the build process unnecessarily since it requires downloading data
from the unicode web site. Further, there is no logical need to regenerate
it at build time any more than there is a need to rebuild
LineBreakUtils.java.

None of the rationale you provide below are compelling. So I object to this
proposed change.

On Thu, Nov 22, 2012 at 2:52 AM, Vincent Hennebert <vh...@gmail.com>wrote:

> Hi,
>
> I’ve recently stumbled upon this o.a.f.complexscripts.bidi.BidiClass
> that appears to be generated. I think it should be moved to the
> build/gensrc directory. Reasons are:
> • It doesn’t need to be understood and maintained by developers in the
>   same way as other classes. Changes will be made to the generator and
>   not this class. Therefore it doesn’t need to follow the usual style
>   and readability rules.
> • Despite the warning at the beginning of the class, there is still
>   a risk that it is accidentally modified during some refactoring
>   session.
> • There is also a risk that the generator is modified and this class not
>   re-generated, introducing a discrepancy that may take some time to be
>   tracked down.
>
> Like any other generated file, I think the generation should happen at
> any clean build.
>
> Any objection?
>
> Thanks,
> Vincent
>