[Bug 524707] Review Request: chronojump - A measurement, management and statistics sport testing tool

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=524707


Michel Alexandre Salim <michael.silvanus at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+




--- Comment #8 from Michel Alexandre Salim <michael.silvanus at gmail.com>  2009-10-04 17:05:38 EDT ---
Just one thing -- rather than creating a tempfile, sed-ing into it and then
renaming the temp, why not use sed -i (in-place) instead?

Also, sed can use different separator tokens, so when the string you want to
replace is full of slashes, use something else -- say pipe -- instead

sed -i 's|share/doc/chronojump|share/doc/chronojump-%{version}|g' src/utils.cs

Otherwise, you might want to use $(mktemp) rather than `mktemp` -- backquote is
a bash-ism (I just noticed it being frowned on in the Python merge review).

The program's output is *very* noisy, but I guess that's more of an upstream
problem.

But you can make these changes when you commit the files to CVS -- everything
else looks good.

APPROVED

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.