Applying Coding Standards
Cezary
First post here, so please forgive me if I broke a rule or more.
I like the first point of the coding standards from the following
thread:
http://groups.google.com/group/testlink-dev/web/coding-standard-for-testlink?hl=en
1.1.1. Write your code for humans and not for computers!
I know that Testlink as a project was started when many people still
used notepad to write PHP applications. I believe TestLink is a great
tool that does the job. Unfortunately, for a tool helping achieve
quality, the quality of the source code is very questionable for
today's standards and expectations.
Especially for corporations, which are amoung the target audience for
TestLink. And it has reached the point, where even contributing is a
problem - both for technical and for psychological reasons.
There are two keys issues: whitespaces and source control, and both
are tied together.
Consider this scenario:
1. user checks out a release version of TestLink from sources (SVN) or
from the github repository (cool, thanks!)
2. CVS keywords are expanded, which mark files as modified :(
3. user creates patches in a very popular editor (Vim) and uses a
modern source control (e.g. git: git-svn)
4. On every save, the editor fixes whitespaces by:
1. making newlines consistent (either DOS or UNIX)
2. removing trailing whitespaces
3. fixing tabs/spaces and possibly indenting the file
4. fixing missing newline at EOF
And these options are common settings if not default plugins or even
default
settings.
5. When showing differences with a VCS, whitespaces are usually set to
be ignored or fixed, although git for example, by default highlights
whitespace 'errors' (such as mixing tabs and whitespaces, DOS
newlines, missing newline ad EOF, etc).
6. Now we have a 15 line 'diff' among files that is actually >500kB
patch!
7. Of course, rebasing with trunk before submitting a patch may become
a nightmare.
8. In order to make the patch minimal, the user has to merge
whitespaces(!!) which is very time consuming if even attempted !! You
cannot generate a "diff" ignoring whitespaces and apply in on clean
checkout, because it won't apply cleanly because of whitespace
changes. And no, "patch -l" doesn't work, I tried...
So, anyone that isn't a committer and wants to work on nice, indented
code, and works on custom patch sets is basically, well, "punished"!
And I know this isn't the intention of TestLink developers.
Or, you can do the alternative to the above:
- use vi in binary mode(!!), turn of ignoring whitespaces for diffs,
hack around eof fixing
- reconfigure git to allow broken whitespaces (around 10 options)
- get another job
And rebasing also means you have to deal with expanded keywords in
sourcefiles, which means that when merging, every file is detected as
changed.
I know that everyone with commit access working with SVN doesn't have
these problems, but TestLink is a huge project and accidentally
dumping contributors who have setups adhering to coding standards will
hurt both developers and TestLink users in the long run.
Examples in TestLink
====================
I found all the above problems in Testlink. I wouldn't mention the
cases if they were consistent among all files (e.g. DOS newlines
everywhere).
Indent is also broken in some places, making code confusing and
harder to debug.
As a psychological effect, this everyone that views the codes
perceives TestLink as a low quality product and abandon contribution
faster. I have trouble convincing anyone of the contrary.
Solution
========
I here now humbly request that the coding standards for Testlink are
extended (whitespaces) and applied globally for all Testlink sources.
Specifically:
- unix newlines (my suggestion, to work with git, popular for
today's web applications)
- 2 spaces for indent (my suggestion, popular nowadays for web
development)
- newlines at EOF (since probably most potential contributors will
run into warnings from editors and merge tools)
- remove trailing whitespaces (cause unnecessary noise when you want
to honor whitespaces, but your tools still clean them up
automatically)
- use a beautifier for PHP (I suggest PEAR style) - at least to fix
code block indending
- suggest a commit hook with tools, that could do the above
automatically (long term goal)
Please, these changes cost maintainers almost nothing, and mean the
world to contributors. I can even do everything myself, but would you
want to merge a >2MB (guessing) patch containing 99% of newline
changes? I would want to spare >you the trouble if I could.
Thankyou so much for your consideration and best wishes for you and
your families,
Cezary
Francisco Mancardi
as current Team Leader of this development, I would like to set clearly
that we are ready to consider any kind of suggestion, analise it and
see if it fir or not our expectations.
Considering that:
- one of worst defect of mail is lack of 'tone', i.e. just dry words
- cultural differences related to different country of origin, create
misunderstandings
I have to say that ideas present in this email can be to a certain extent
good, but the way used to present it have not produced a positive effect.
(my poor) Suggestions for next time you plan to write something similar:
1. do not write to this list, but ask to the list how to contact core
developers/team leader
2. ask information, if you need it to understand better our choices,
instead of making assumptions.
Probably we have had and still have good reasons to proceed in this or
that way.
We are always looking for contributions, but because we lead this
project we are certain that
we have set the guidelines, contributor has to accept it.
Personally thing that will to contribute can not be reduced just due to
use of this source control system or
that, or by amount of spaces.
Without any intention to start a end-less series of mails.
regards
Francisco Mancardi
Erik Eloff
I'm sorry you feel it's difficult to contribute to testlink.
I understand your issues and have been sharing some of them from time
to time. I've tried to introduce git as a modern source control, but
the old cvs habits were to strong for that change at the time. Also I
haven't noticed lots of pressure from the community to move forward to
git.
To be able to work on testlink I setup my own git repo worked and
committed locally. Whenever something was to be committed to the cvs
head I used "git diff" to get a rather clean patch that would apply to
the cvs tree. I use to clean up whitespace manually on the lines I
changed; doing whitespace fixes globally would, as you say, create a
mess to big to handle :(
At the time Francisco is the person reviewing and merging most of the
code. Contributions are attached in the bugtracker (not as diffs, but
as complete files zipped together), then Francisco downloads, merges
and commits to cvs. I don't know how exactly he is doing this but
somehow he manages it.
Thanks for your offer to fix the white-space issues. I'd certainly
welcome it. However I think a discussion is needed to decide a
reasonable "white-space standard" that works well with respect to
choice of text editor and platform before any work is done.
I'd also like to hear what Francisco thinks about this...
Regards,
Erik Eloff
2011/1/13 Cezary <cezary....@gmail.com>:
> --
> You received this message because you are subscribed to the Google Groups "testlink-dev" group.
> To post to this group, send email to testli...@googlegroups.com.
> To unsubscribe from this group, send email to testlink-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/testlink-dev?hl=en.
>