Bug 107941 – Add support to upload to gist in webkit-patch.

• Component: Tools / Tests
• Status: NEW

[Alexis Menard (darktears)]

It would be very handy to upload a diff to gist directly from webkit-patch.

webkit-patch upload-gist for example which will output the link to the terminal and add it to the clipboard.

We could rely on the gist script already provided by github.

[Eric Seidel (no email)]

Should be trivial to add, and likely useful to the project.

[Adam Roben (:aroben)]

http://developer.github.com/v3/oauth/#create-a-new-authorization and http://developer.github.com/v3/gists/#create-a-gist should be pretty much all you need.

[Gergő Balogh]

Created attachment 216015 [details]
patch

A new AbstractStep and AbstractSequencedCommand command created (UploadAnonymousGist and Gist) which are implement the given feature.

[WebKit Commit Bot]

Attachment 216015 [details] did not pass style-queue:

Traceback (most recent call last):
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 44, in <module>
    from webkitpy.tool.main import WebKitPatch
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/main.py", line 41, in <module>
    from webkitpy.tool import commands
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/__init__.py", line 4, in <module>
    from webkitpy.tool.commands.analyzechangelog import AnalyzeChangeLog
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/analyzechangelog.py", line 39, in <module>
    from webkitpy.tool import steps
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/__init__.py", line 65, in <module>
    from webkitpy.tool.steps.uploadanonymousgist import UploadAnonymousGist
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py", line 31, in <module>
    import requests
ImportError: No module named requests


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gergő Balogh]

Created attachment 216027 [details]
patch

A new AbstractStep and AbstractSequencedCommand command created (UploadAnonymousGist and Gist) which are implement the given feature.

Request lib replaced with urllib2.

[WebKit Commit Bot]

Attachment 216027 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy/tool/commands/__init__.py', u'Tools/Scripts/webkitpy/tool/commands/gist.py', u'Tools/Scripts/webkitpy/tool/steps/__init__.py', u'Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py']" exit_code: 1
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:2:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:6:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:16:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:38:  expected 2 blank lines, found 1  [pep8/E302] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:41:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:43:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:44:  too many blank lines (2)  [pep8/E303] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:52:  trailing whitespace  [pep8/W291] [5]
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:47:  whitespace after '('  [pep8/E201] [5]
Tools/Scripts/webkitpy/tool/commands/gist.py:39:  expected 2 blank lines, found 1  [pep8/E302] [5]
Tools/Scripts/webkitpy/tool/commands/gist.py:46:  whitespace before ']'  [pep8/E202] [5]
Total errors found: 11 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gergő Balogh]

Created attachment 216028 [details]
patch

style fixes

[WebKit Commit Bot]

Attachment 216028 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy/tool/commands/__init__.py', u'Tools/Scripts/webkitpy/tool/commands/gist.py', u'Tools/Scripts/webkitpy/tool/steps/__init__.py', u'Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py']" exit_code: 1
Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:52:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peter Gal]

Comment on attachment 216028 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216028&action=review

> Tools/Scripts/webkitpy/tool/commands/gist.py:2
> +# Copyright (c) 2009, 2011 Google Inc. All rights reserved.
> +# Copyright (c) 2009 Apple Inc. All rights reserved.

Is this really correct?

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:1
> +# Copyright (C) 2010 Google Inc. All rights reserved.

Is the copyright information really correct?

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:42
> +    def options(cls):
> +        return AbstractStep.options()

This is not needed AbstractStep is the base class for this.

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:59
> +            req = urllib2.Request(url, gist_data)
> +            res = urllib2.urlopen(req)

Please do not abbreviate the variable names: 'req' -> 'request' and 'res' -> 'result'

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:60
> +            new_gist = json.loads(res.read())

Are we really sure that the result is always json?

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:66
> +                exit(1)

I think we should use the base class' _exit method for this.

> Tools/Scripts/webkitpy/tool/steps/uploadanonymousgist.py:68
> +            print('Something went worng')

Typo: 'worng'

[Gergő Balogh]

Created attachment 217527 [details]
patch fix

The gist api claim the response (if any) will be in json if theres is any error.

[Gergő Balogh]

Created attachment 218057 [details]
patch fix

[Csaba Osztrogonác]

Comment on attachment 218057 [details]
patch fix

The idea is good, but it needs some fine tuning.

https://gist.github.com/anonymous/888fc4688c4827ff2b98#file-diff
--> filetype isn't set to diff