New issue
Advanced search Search tips

Issue 899415 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

blink/renderer presubmit should allow features:: namespace

Project Member Reported by ericrk@chromium.org, Oct 26

Issue description

As written, the allowed namespace/classes list in
audit_non_blink_usage.py does not allow for classes in the features::
namespace to be used.

These are already frequently used in blink/renderer, however as they
are almost always preceded by base::FeatureList::IsEnabled, and the
checker only checks the first match on a line, they were previously
ignored.

This seems like an oversight as the features:: namespace is
already in common use in blink/renderer, and base::Feature.* is
allowed already. Adding in features::.+ as well.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab0e67a24d2bfab83c2580c853600314bd2d7a47

commit ab0e67a24d2bfab83c2580c853600314bd2d7a47
Author: Eric Karl <ericrk@chromium.org>
Date: Mon Oct 29 21:09:40 2018

Allow features::.+ in blink/renderer

As written, the allowed namespace/classes list in
audit_non_blink_usage.py does not allow for classes in the features::
namespace to be used.

These are already frequently used in blink/renderer, however as they
are almost always preceded by base::FeatureList::IsEnabled, and the
checker only checks the first match on a line, they were previously
ignored.

This issue was encountered when trying to add a longer features::.+
element that was wrapped to the next line. However, moving any
existing features::.+ string to its own line will trigger this
issue.

This seems like an oversight as the features:: namespace is
already in common use in blink/renderer, and base::Feature.* is
allowed already. Adding in features::.+ as well.

Bug:  899415 
Change-Id: I56d453e102988b463d91850d5aa843c93c274517
Reviewed-on: https://chromium-review.googlesource.com/c/1303395
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603627}
[modify] https://crrev.com/ab0e67a24d2bfab83c2580c853600314bd2d7a47/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py

Components: -Blink Blink>Internals
Status: Fixed (was: Started)
Seems done?

Sign in to add a comment