[Amarok] fix: create .../scripts/ directory if it doesn't e

Jakob Kummerow jakob.kummerow at googlemail.com
Wed Nov 4 11:32:42 CET 2009


> This variable should be "const QString", unless it's being modified
> later on. Also, please don't use cryptic names. Call it
> "relativePath", then it will be easier to understand.

Done.

>> +    QDir dir; // only needed as a dummy because .exists() and .mkpath() aren't static
>
> This comment does not make sense. Either you use the object (which you
> do), then it's not a dummy, or you don't. Whether the methods are
> static is clearly visible from the API.

I meant that it's a dummy in the sense that none of its properties are
ever set or read. I could just as well have used "QDir::exists()" and
"QDir::mkpath()", *if* those two were declared static.
Anyway, never mind, I've removed the comment. Don't want to cause any
confusion or irritation ;-)

> Please respect our coding style conventions.

Yeah, sure. I was focused on what the code does, not what it looks
like, so I forgot. Fixed.

> PS:
> I'm being a little pedantic here because you're a new contributor.
> Learning early makes it easier to break bad habits :)

That's OK.
The thing is that during the past few years, I've written quite a lot
of Java code and almost no C(++) at all, so I'm used to whatever is
standard practice in Java. But I'm already beginning to appreciate the
clearer and more relaxed visual appearance of having spaces inside
parentheses :-)


Jakob


More information about the Amarok-devel mailing list