Add WPTEXPORT footer automatically in DepsUpdater and imported/PRESUBMIT

third_party/WebKit/LayoutTests/imported/PRESUBMIT.py

+# Copyright (c) 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import re
+
+
+def PostUploadHook(cl, change, output_api):  # pylint: disable=C0103

[qyearsley, 2016/11/17 00:23:27]

For pylint disable pragma, you can use the symbolic name of the pylint rule.
Also optionally you can explain why it's necessary, e.g.
# Function name dictated by depot_tools/presubmit_support - pylint:
disable=invalid-name

+    """git cl upload will call this hook after the issue is created/modified.

[foolip, 2016/11/17 09:59:14]

What about things in LT/imported/csswg-tests and wpt_automation? Can this script
avoid changing those CLs?

[jeffcarp, 2016/11/17 20:49:58]

Yeah it sounds possible to filter those out (in theory)

+
+    This hook adds WPTEXPORT=true to a CL if it doesn't already exist. This is

[foolip, 2016/11/17 09:59:14]

Whether this is useful depends on what you intend for the export script to do.
If it finds a change in LT/imported/wpt/ with no WPTEXPORT flag, what will it
do? That will still happen, e.g. if there's a sheriff revert using
NOPRESUBMIT=true.

Maybe that situation would require human intervention?

[qyearsley, 2016/11/17 17:44:50]

I think I'd expect that the default would be for the exporter to try to export
if there's no WPTEXPORT flag given.

In this case, another way that this could be done would be to not add the flag
in the presubmit, but check for a flag "NOEXPORT=true" added to import CLs, and
optionally added to other CLs. That way is pretty consistent with the way that
NOTRY and NOPRESUBMIT are, as a means of disabling a default behavior.

[jeffcarp, 2016/11/17 20:49:58]

That sounds like a cleaner way of implementing this behavior - and it requires
no modifications to the presubmit process, only modifications to the export
script.

[foolip, 2016/11/17 20:52:34]

Yeah, that sounds good. I was thinking WPTEXPORT=false to avoid the negation,
but NOEXPORT=true would fit nicely in with existing flags, so let's do that. Not
sure if there's a "flag owner", but might be worth checking who added some
existing flags and if they think the name is good.

+    to better help the WPT export script identify exportable commits.

[qyearsley, 2016/11/17 00:23:27]

The second sentence could be changed to note what WPTEXPORT signifies, e.g.
"WPTEXPORT indicates whether or not the commit should be automatically exported
to the web-platform-tests repo."

+    """
+    rietveld_obj = cl.RpcServer()
+    description = rietveld_obj.get_description(cl.issue)
+    if re.search(r'^WPTEXPORT=.+', description, re.M | re.I):
+        return []
+
+    new_description = description
+    new_description += '\nWPTEXPORT=true'
+    rietveld_obj.update_description(cl.issue, new_description)
+
+    return [output_api.PresubmitNotifyResult((
+        'Automatically added WPTEXPORT=true flag due to files being modified '
+        'in LayoutTests/imported and no existing WPTEXPORT flag found.'))]