On 21/10/2011 15:13, Glenn Adams wrote:
Chris,
Hi Glenn,
I would really like to see an acknowledgement from Glenn that there
are some imperfections that need addressing.
I wasn't aware I had given anyone the impression of presenting a
perfect submission. In fact, one of my favorite quotes is Voltaire's
/le mieux est l'ennemi du bien/ "the best [perfect] is the enemy of
the good", which a former colleague Charlie Sandbank (now deceased)
used to love to cite at least once a day in ITU committee meetings.
Many thanks for taking the time to write this long e-mail. I do
appreciate it. I reached this impression because I see Vincent and Peter
giving feedback but I've yet to see any of the suggestions actioned. It
could be that some of their suggestions aren't appropriate, but I do
believe some of them are good points.
My coding philosophy is by and large based on a step-wise refinement
process:
(1) make it (some subset of possible features) work
(2) make sure its correct (and regression testable) - or if following
BDD, then do this first
(3) make it (more) understandable/maintainable, i.e., re-factor,
improve comments, documentation, etc
(4) optionally, make it faster and smaller
(5) optionally, add more features
(6) go to (1)
Right now, I've finished steps (1) and (2) to a certain degree for an
initial set of features, a degree that I think is sufficient to merit
moving it into trunk. I have not yet seriously addressed (3) through
(6). It would seem strange to expect that all these points have been
addressed at this point in the process.
Thanks for clarifying. Just to be clear, I didn't mean to say that
reaching the end of development was a pre-requisite to merging to trunk.
It just seemed like a major milestone and therefore seemed like a
suitable prompt for the opening of a discussion about our concerns.
If this work goes into the trunk, it will provide greater exposure to
help drive and prioritize the remaining steps. There are trade-offs in
time and money about how to spend my effort on steps (3) to (6). I
have a well defined set of issues already waiting for item (5) [1],
so, post merge, I can spend my time on these features or work on (3)
or (4), or could attempt to divide my time between them. The bottom
line is that it is a process that started some time ago and will
continue for an indefinite time into the future. The current request
for merging is just one step in that process.
That makes sense to me.
I expect to be maintaining this code and feature set for the
indefinite future, according to the desires of my sponsor, Basis
Technologies, who has a definite interest in the production use of an
internationalized FOP, as well as others who have expressed a similar
interest. As a consequence, there is little chance that any other FOP
dev is going to have to work on this code any time soon. Of course, if
they want to do so, I would certainly welcome community contributions
to additional features, testing, optimization, documentation, etc., in
this area. I have no personal need to *own* this code if others wish
to help, and I certainly encourage it; however, at the same time, I do
have a sponsor who is willing to continue investing in improving FOP
in this area, and that willingness should not be discounted.
Since Thunderhead also needs this feature we are willing to invest some
time into it too. Currently my team are telling me it would take 9
person months to port this code into our branch of FOP, partly because
of some merge conflicts, but also partly because we are not comfortable
with some aspects of the code as it has already been pointed out. The
estimate would include the time to refactor long files into small ones,
deal with the variable names and restructuring the code.
[1] http://skynav.trac.cvsdude.com/fop/report/1
Regarding the comments about line length, file length etc., I will
note that, at least with line length, I have maintained the existing
(but arbitrary) limit of 100 in existing files. In the case of new
files, I have chosen to use a longer line length that works better for
my development process. I use GNU EMACS on a wide-screen MacBook Pro
that has an effective width of 200 columns, and which, when this limit
is exceeded, wraps the line (on display only). I find that arbitrary
line lengths like 100 curtail my coding style, and as I've been coding
for 40+ years, it's a pretty well established style (though back in
the days when I wrote Fortran IV using an IBM 026 card punch
<http://en.wikipedia.org/wiki/IBM_026#IBM_024.2C_026_Card_Punches>, I
had to stick with 80 columns). [If line length followed Moore's Law,
we would be using lines of length 1760, starting from 1967 with 80
columns.]
I personally don't have a problem with line length, but I think the File
length is a small issue that we would like to address. I think the code
would be easier to maintain if the larger classes were broken into
several smaller classes.
By the way, I would note that the FOP Coding Conventions [2] do not
specify a maximum line length. In searching through the FOP DEV
archives, I also don't see an explicit discussion about line length
(though I may have missed it). What I do see is the initial
introduction of the checkstyle target by Jeremias in 2002 [4], where
it appears he basically adjusted the checkstyle rules to pass most of
the extant code at that time.
[2] http://xmlgraphics.apache.org/fop/dev/conventions.html
[3]
http://marc.info/?l=fop-dev&w=2&r=1&s=%2Bcheckstyle+%2Bline+%2Blength&q=b
<http://marc.info/?l=fop-dev&w=2&r=1&s=%2Bcheckstyle+%2Bline+%2Blength&q=b>
[4] http://marc.info/?l=fop-dev&m=103124169719869&w=2
<http://marc.info/?l=fop-dev&m=103124169719869&w=2>
My opinion about the various length limits (parameter count, method
length, file length, line length) as reported by checkstyle is that
these should interpreted as guidelines, and not hard rules. In the
case of existing files, it makes sense to attempt to adhere to them
when possible (and I have done this), while recognizing that some
exceptions are inevitable. In the case of new files, I agree they are
reasonable targets, but I would not readily agree to slavish
adherence. Some latitude should be afforded to individual coders,
particularly when they are adding a substantial amount of code in new
files.
I think that is a fair statement, but breaking the rules should be the
exception not the rule. Otherwise we need to review the rule as a
community or consider another code structure.
Regarding identifier length, neither the FOP coding conventions [2]
nor checkstyle indicates or checks for any kind of limitation; so I
will respectfully decline to change my code based on other author's
subjective notions of what is necessary or sufficient. If folks want
to have a discussion about this in order to reach a consensus, then I
would not object to adhering to a consensus that emerges; but IMO,
there are more important things to do than hammer out such a consensus
in an objectively verifiable rule. What I will agree to do is add
(over a reasonable period of time) comments at the point of defining
all static, instance, or local variables that adequately spell out the
meaning of any 'short identifier' save for the obvious indexers, i, j,
k, etc, and basic type variables (String s) and enumerators
(incidentally, where would one draw the line with an objective rule?).
It's true that there is currently no rule about this in the coding
conventions. I think the community should seriously consider adding such
a rule. I think the reason for it absence is that no one used the short
identifiers on mass before. I agree variables used as iterators are an
exception to the rule; Peter, Vincent nor I are trying to suggest that
i,j,k are not allowed as iterators. I think that is a well known
convention. Here is an example of what we mean:
int gi = 0;
Where gi really means glyphIndex. In this case we believe the variable
should be named "glyphIndex"
I appreciate your commitment to add comments to short identifiers
declarations, so at least it will be easier for the team to translate
the short variables to longer equivalents. Just so we are clear on what
you propose, do you mean this:
int gi = 0; // Glyph Index
Thanks,
Chris
Regards,
Glenn
On Fri, Oct 21, 2011 at 5:50 PM, Chris Bowditch
<[email protected] <mailto:[email protected]>> wrote:
On 21/10/2011 09:36, Simon Pepping wrote:
Hi Simon,
I am pleased to learn that you are also in need of this new
functionality.
I share some of Vincent and Peter's concerns about technical
points of
the code. On the other hand, this is the only implementation of
complex scripts we have, created by Glenn, in the style of
Glenn. It
is an initial implementation, and it is normal that it requires
further work, maybe even design changes to make it more
flexible. Does
keeping it in a branch make that further work easier? Merging
it into
trunk will enhance its visibility, and make it available to more
users.
I'm not opposing the merge, I simply saw it as an appropriate
milesone at which to open the debate on our concerns. It feels
like we are making some progress here, so thanks for helping the
debate along. I would really like to see an acknowledgement from
Glenn that there are some imperfections that need addressing. If I
saw that then I would give my full backing, but even without that
I would vote +0 for the merge for the reasons you highlight above.
Thanks,
Chris
Simon
On Thu, Oct 20, 2011 at 02:02:10PM +0100, Chris Bowditch wrote:
On 19/10/2011 19:32, Simon Pepping wrote:
Hi Simon,
I think you misunderstood my mail. I don't want to stop
the merge. I
simply thought it was an appropriate time to discuss some
concerns
that Vincent and Peter had identified. You are preaching
to the
converted about the need for supporting Complex scripts.
It is an
urgent requirement for us too.
If we don't discuss our concerns over the code now, then
when do we
discuss it?
Vincent and Peter will be replying to this thread shortly
and will
outline their primary concerns then.
Thanks,
Chris
