Re: [PATCH] extfs scripts fixes



Hello!

Some comments as I'm applying your patch.  I think I have spent more time
on it than you (it's not meant as offense, just a thought).  It would be
very helpful if you provide ChangeLog entries with your patches in the
future.

I don't like this part:

 # (cp -p) to preserve date, but $2 is dated now!
-    cp -p $3 "$3.dir/$2"  
+# [-p breaks permissions, removed - alpha] 
+    cp $3 "$3.dir/$2"  

It's unfriendly to the future developers to leave the comments in such 
state.  There is no need to leave comments about the code that you are 
removing.

If we check uzoo, there are such comments about it being "dangerous".  If 
you didn't remove that comment, then probably you didn't notice it?  And 
If you didn't notice it, what's the point in adding more irrelevant 
comments about what the script used to do?

Also let's stop putting names into the comments.  We cannot expect those
who change the code to keep those comments updated and add their names -
it's an additional burden.  ChangeLog and "cvs annotate" is sufficient.  
Not to mention that "alpha" and even "pavel" is not sufficient to locate
the author of the comment.

Finally, I wonder if you bothered to read "man cp".  I did.  By the way,
there is one thing that the manual doesn't say.  Some systems make "cp"
and alias to "cp -i", so it can be interactive.  Using "cp -f" makes it
non-interactive again.  That's why configure script and makefiles always
use "cp -f", not just "cp".

-- 
Regards,
Pavel Roskin




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]