Bug 28669 – make-script-test-wrappers should be executable

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Cameron McCormack (:heycam)]

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.

[Cameron McCormack (:heycam)]

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.

[Cameron McCormack (:heycam)]

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.

[Darin Adler]

(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.

[Cameron McCormack (:heycam)]

(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.

[Darin Adler]

(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.

[Cameron McCormack (:heycam)]

(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.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

r47764