Mailinglist Archive: opensuse-buildservice (351 mails)

< Previous Next >
Re: [opensuse-buildservice] osc submitreq accept broken
  • From: Dirk Mueller <dmueller@xxxxxxx>
  • Date: Wed, 16 Jul 2008 11:52:11 +0200
  • Message-id: <200807161152.12072.dmueller@xxxxxxx>
On Tuesday 15 July 2008, Klaas Freitag wrote:

anyway, I think it might be better to just use the perl builtin
operator -T for it (and all the other possible bugs still lurking in 5
lines of perl).
Sorry, but this is the kind of code which makes it ugly.

Using -T makes it ugly or not using -T makes it ugly?

If the first: thats why I didn't remove the isascii() function. the
implementation of isascii() should be irrelevant as long as you, when you see
a usage of the function in the code somewhere else know what it does. if you
don't know, a comment at the function header, that describes assumptions,
result and purpose of a function helps a lot more than having to first
understand the purpose of a function by reading it. It is for example not
immediately clear that the isascii() function actually checks for "not binary"
and not for ASCII-ness. A latin1 encoded file would true for isascii() even
though it is anything but ascii.

None of those things were changed or made worse by my commit. I also disagree
that replacing code that was obviously never tested (I've fixed 3 bugs in the
total 6 lines of perl!) with a builtin function is inherently wrong or making
things more ugly. Instead it wrapped an ununderstandable perl builtin function
detail in a descriptive function name. (which I think is not descriptive, but
I don't own the code, I just want it to work :) ). Using builtins is probably
also faster than a openly coded regular expression over a long binary string,
but performance shouldn't be a concern here.

Third: lets not start coding style discussions when it comes to OBS backend,
mkay? functions that are more than 200+ lines long, that are not modularized,
commented, are using quite a big set of perl tricks everywhere, use and update
global variables at random places while expecting them to never change (due to
longrunning iterators over them), do not have clear error recovery paths and
not even testcases is not somethen were we should start discussing about a 6
line helper function IMHO.

Anyway: I accept that the undefinedness of -T's exact way of working is
probably making it a bad candidate for usage here, even though I would prefer
to use it for the sole reason of being engineered, tested and maintained
elsewhere.

Greetings,
Dirk
---------------------------------------------------------------------
To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@xxxxxxxxxxxx
For additional commands, e-mail: opensuse-buildservice+help@xxxxxxxxxxxx

< Previous Next >