Bug 28669 - make-script-test-wrappers should be executable
Summary: make-script-test-wrappers should be executable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-23 05:47 PDT by Cameron McCormack (:heycam)
Modified: 2009-08-25 16:26 PDT (History)
2 users (show)

See Also:


Attachments
Patch to make these two scripts executable (1005 bytes, patch)
2009-08-23 16:59 PDT, Cameron McCormack (:heycam)
darin: review+
eric: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2009-08-23 05:47:00 PDT
WebKitTools/Scripts/make-script-test-wrappers should be executable, like nearly all other files in WebKitTools/Scripts/ are.  update-sources-list.py probably should be executable too.
Comment 1 Cameron McCormack (:heycam) 2009-08-23 05:56:28 PDT
Ah, prepare-ChangeLog doesn't work if the only changes I've made are to properties:

fox:~/svn/WebKit cam$ ./WebKitTools/Scripts/prepare-ChangeLog 
  Running status to find changed, added, or removed files.
 M     WebKitTools/Scripts/update-sources-list.py
 M     WebKitTools/Scripts/make-script-test-wrappers
  No changes found.
Comment 2 Cameron McCormack (:heycam) 2009-08-23 16:59:15 PDT
Created attachment 38461 [details]
Patch to make these two scripts executable

I manually typed in the ChangeLog entry, since prepare-ChangeLog wasn't being cooperative.  Hopefully I got the format right.
Comment 3 Darin Adler 2009-08-23 22:14:18 PDT
(In reply to comment #1)
> prepare-ChangeLog doesn't work if the only changes I've made are to
> properties

Right, it doesn't consider property changes as something it should list in the change log.
Comment 4 Cameron McCormack (:heycam) 2009-08-23 22:18:43 PDT
(In reply to comment #3)
> Right, it doesn't consider property changes as something it should list in the
> change log.

Should it?  It might make generating patches like this one easier if prepare-ChangeLog could be used.  If so, I can file a bug and supply a patch for prepare-ChangeLog if you like.
Comment 5 Darin Adler 2009-08-23 22:23:54 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Right, it doesn't consider property changes as something it should list in the
> > change log.
> 
> Should it?  It might make generating patches like this one easier if
> prepare-ChangeLog could be used.  If so, I can file a bug and supply a patch
> for prepare-ChangeLog if you like.

Sure, it would be OK.

As with newly-added files we could consider pre-adding a comment to the generated file to say what the change is. For example, changing the state of the executable flag is one of the most common changes and it would be good to do it automatically.

On the flip side, when changing flags on tons of files, having a list of the files with flags changed is probably not helpful in the way that listing files with contents changed is in typical patches. I have a slight fear that we'll have change log entries that are largely vacuous. Contributors don't seem to treat the change log as a communication tool as much as I'd like; those lists of files and functions are supposed to be for human consumption, and folks should read them and change anything that does not seem helpful.

But yes, that would be a nice thing to fix.
Comment 6 Cameron McCormack (:heycam) 2009-08-24 00:38:47 PDT
(In reply to comment #5)
> As with newly-added files we could consider pre-adding a comment to the
> generated file to say what the change is. For example, changing the state of
> the executable flag is one of the most common changes and it would be good to
> do it automatically.
> 
> On the flip side, when changing flags on tons of files, having a list of the
> files with flags changed is probably not helpful in the way that listing files
> with contents changed is in typical patches. I have a slight fear that we'll
> have change log entries that are largely vacuous. Contributors don't seem to
> treat the change log as a communication tool as much as I'd like; those lists
> of files and functions are supposed to be for human consumption, and folks
> should read them and change anything that does not seem helpful.
> 
> But yes, that would be a nice thing to fix.

Filed https://bugs.webkit.org/show_bug.cgi?id=28675 for this.

Agreed with the chance for creating large, useless ChangeLog entries, but hopefully contributors will use common sense and pare them down if necessary.
Comment 7 Eric Seidel (no email) 2009-08-24 11:26:19 PDT
Comment on attachment 38461 [details]
Patch to make these two scripts executable

Since the commit-queue is currently running from a git checkout, it won't be able to handle this.  Someone will have to land this manually.
Comment 8 Eric Seidel (no email) 2009-08-25 16:26:59 PDT
r47764