Summary: | Unification of using null device in perl scripts | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, ddkilzer, eric, mrowe | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 31840 | ||||||||
Attachments: |
|
Description
Csaba Osztrogonác
2009-10-20 08:47:23 PDT
Created attachment 41507 [details]
proposed patch
Comment on attachment 41507 [details]
proposed patch
I believe our style is to put spaces around '.' operators.
Also, I'm surprised that sometimes you use local variables to hold $devNull and sometimes you make direct calls to File::Spec->devnull().
Is devnull() always a string path?
(In reply to comment #2) > I believe our style is to put spaces around '.' operators. Yes, you're right, sorry. I will correct it. > Also, I'm surprised that sometimes you use local variables to hold $devNull > and sometimes you make direct calls to File::Spec->devnull(). I told I prefer using File::Spec->devnull() everywhere, but I have no idea how could I use it directly in `...`. Accordingly I use local variable same as in svn-create-patch originally. Have you got better idea to solve this problem? > Is devnull() always a string path? You can find devNull() here: http://perldoc.perl.org/File/Spec.html "Returns a string representation of the null device." It works correctly, we used it previosly in WebKit scripts. (In reply to comment #2) > I believe our style is to put spaces around '.' operators. Yes, you're right, sorry. I will correct it. > Also, I'm surprised that sometimes you use local variables to hold $devNull > and sometimes you make direct calls to File::Spec->devnull(). I told I prefer using File::Spec->devnull() everywhere, but I have no idea how could I use it directly in `...`. Accordingly I use local variable same as in svn-create-patch originally. Have you got better idea to solve this problem? > Is devnull() always a string path? You can find devNull() here: http://perldoc.perl.org/File/Spec.html "Returns a string representation of the null device." It works correctly, we used it previosly in WebKit scripts. (In reply to comment #2) > (From update of attachment 41507 [details]) > I believe our style is to put spaces around '.' operators. Updated. > Also, I'm surprised that sometimes you use local variables to hold $devNull and > sometimes you make direct calls to File::Spec->devnull(). I modified bisect-builds, we can use exec instead of backstick operator. But I have no idea what should we do with backstick of svn-create-patch: print `svn cat ${sourceFile} | diff -u $devNull - | tail -n +3`; We can't use system or exec insted of backsticks. I think we shouldn't rewrite this code at any price. Created attachment 41876 [details]
proposed patch
CCing perl folks, as I am not one. Thx for review. Sending WebKitTools/ChangeLog Sending WebKitTools/Scripts/VCSUtils.pm Sending WebKitTools/Scripts/bisect-builds Sending WebKitTools/Scripts/resolve-ChangeLogs Sending WebKitTools/Scripts/run-iexploder-tests Sending WebKitTools/Scripts/run-jsc Sending WebKitTools/Scripts/run-mangleme-tests Sending WebKitTools/Scripts/run-webkit-tests Sending WebKitTools/Scripts/webkitdirs.pm Transmitting file data ......... Committed revision 50080. |