[PATCH] D73075: [utils] Add initial llvm-patch helper to manage patches.

[Florian Hahn via Phabricator via llvm-commits]

fhahn abandoned this revision.
fhahn added inline comments.


================
Comment at: llvm/utils/llvm-patch:62-63
+        data = json.loads(response.read())
+        if not data['result']:
+            print('ERROR: {}'.format(data['error_info']))
+        return data['result']
----------------
paquette wrote:
> paquette wrote:
> > If `data['result']` is an empty string, this will still return something. Should there also be a check for if `data['result']` is empty?
> - If `data['result']` is an empty string, this will still return something. Should there also be a check for if `data['result'] = ''`?
> - Is there only data in `'error_info'` when `data['result']` is empty? Would it make more sense to just check if `data['error_info']` has something in it?
> - `sys.exit(1)` on error?
changed it to use data.get('result', '') and then checking the length


================
Comment at: llvm/utils/llvm-patch:77
+    """
+    revision_id = int(args.ID.replace('D', ''))
+
----------------
paquette wrote:
> This will crash if `args.ID` is not an integer.
validated now during argparse, as suggested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73075/new/

https://reviews.llvm.org/D73075