Reland notes #2: The reason the previous reland was reverted was dealt
in a precursor CL (see crbug.com/398274433) and is no longer part of
this reland.
Bug: 397907153
Change-Id: I2ee1871ad10f3d607634085a31eb5c37e61e8dd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6293728
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Auto-Submit: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1424698}
Previously only CheckChangeOnUpload was implemented, which is not great,
as it makes the behavior between `git cl upload` and `git cl presubmit`
different.
As part of implementing CheckChangeOnCommit, also had to fix an error
that was only thrown when the presubmit tests run on Windows, causing
[1] to fail in https://crrev.com/c/6287035 and consequently causing the
CL to be reverted.
Verified the fix on a local Windows checkout, by running
git cl
presubmit --files="ios/*"
[1] https://ci.chromium.org/ui/p/chromium/builders/ci/win-presubmit
Fixed: 398274433
Change-Id: I89843333ed416b88c46ec44010d2efed44053d7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6292271
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1423443}
This reverts commit d8d407d71b.
Reason for revert: might have caused builder failures
crbug.com/398274433
Original change's description:
> [Reland] WebUI: Turn on ESLint JS presubmit checks for ios/ folder.
>
> Reland notes: Limiting the original change to only apply to JS files
> instead of both JS and TS, as there are still TS violations that need to
> be fixed. Also confirmed that the following command passes locally
> `git cl presubmit --files="ios/*.ts;ios/*.js"`
>
> Also adding CheckChangeOnCommit() which was previously erroneously
> missing, causing `git cl presubmit --files=...` to not behave
> equivalently to `git cl upload`.
>
> Bug: 40519637
> Change-Id: I3be6f2a7bf5d89c32d3ae316e2568baa8b509411
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6287035
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Rohit Rao <rohitrao@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1423237}
Bug: 40519637, 398274433
Change-Id: I45eb962de484a6bc60557de3c5f3f2501618acd3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6292786
Owners-Override: Muyao Xu <muyaoxu@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Muyao Xu <muyaoxu@google.com>
Auto-Submit: Muyao Xu <muyaoxu@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1423376}
Reland notes: Limiting the original change to only apply to JS files
instead of both JS and TS, as there are still TS violations that need to
be fixed. Also confirmed that the following command passes locally
`git cl presubmit --files="ios/*.ts;ios/*.js"`
Also adding CheckChangeOnCommit() which was previously erroneously
missing, causing `git cl presubmit --files=...` to not behave
equivalently to `git cl upload`.
Bug: 40519637
Change-Id: I3be6f2a7bf5d89c32d3ae316e2568baa8b509411
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6287035
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1423237}
This CL introduces a new presubmit check to enforce using shared Chrome
iOS colors by requiring all new colors to be added to
ios/chrome/common/ui/colors/ and providing helpful warnings to users
when colors are modified.
Technical implementation:
- Adds CheckNewColorIntroduction presubmit check that monitors colorset
file changes
- Implements error checking for new colors added outside
ios/chrome/common/ui/colors/
- Provides warnings for modified colors
- Adds FormatMessageWithFiles helper for consistent error reporting
Critical conditions:
- Errors: New colors must be added to ios/chrome/common/ui/colors/
- Warnings: Generated for:
- New colors added to ios/chrome/common/ui/colors/ (to check for
duplicates)
- Modifications to existing colors (to ensure shared color usage)
Usage constraints:
- Affects all changes to .colorset/Contents.json files
- Full test coverage included to validate different color modification
scenarios
Change-Id: I01fc85d553be7d6e234bd4d29fc353c55bc26ba3
Fixed: 397741197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6281073
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Benjamin Williams <bwwilliams@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1422462}
This CL adds a presubmit check trying to limit new usage of
NSUserDefaults.
Note that this check provides a WARNING message.
This check does not cover all scenarios and can produce some false
positives (in case the developer uses the pattern code to get
the content of an existing userDefault key).
Bug: none
Change-Id: Ie78b731ff5517a5c2d331494b2e76c66f630c6dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6179973
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Federica Germinario <fedegermi@google.com>
Cr-Commit-Position: refs/heads/main@{#1411606}
This CL adds a presubmit check which checks if new changes in
unit test code can be improved by giving a warning message.
Example of lines that can be improved:
(EXPECT_TRUE or EXPECT_FALSE) +
(isEqualToString: or isEqualToData: or isEqualToArray:)
This CL also fixed the error_message string of:
_CheckHasNoChromeBrowserStateForwardDeclaration
Bug: none
Change-Id: I46a3593c7d9d41c54cdf87986895e295553e01c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5866150
Commit-Queue: Federica Germinario <fedegermi@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1356410}
Adds a PRESUBMIT check to catch forward-declarations of
ChromeBrowserState, which (for the time being) should instead be
done with the forwarding header.
The check for this is intentionally quite narrow.
Bug: 359821340
Change-Id: I143cd68ad37ccb69a03f17014ec3819f6c2fde5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5797169
Auto-Submit: Mark Cogan <marq@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1344010}
This makes CWVPageView.title, CWVBackForwardListItem.title and
CWPageView.lastCommittedUrl nullable so that their Swift bindings
become equal to the WebKit variants.
For Objective C there will be no noticeable change which might be
why it was never noticed.
This also modifies the PRESUBMIT check to allow nullability
in ios/web_view even if it is banned everywhere else.
Bug: 334275652
Change-Id: I957430f556df9a8bfabb1a947e66f188a009eaa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5444561
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/main@{#1288560}
This CL changes the ios/ PRESUBMIT to warn if b/ links are used in
TODOs.
Also fixes some error messages. In the singular, inserts an 'a' for
correctness ("without a bug number" vs "without bug number"). Also now
the parentheses opened with "(expected format is" is now closed before
the final colon.
Bug: 326058403
Change-Id: I863a013120939e3fe20656b064c8d095e30b1223
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5343955
Reviewed-by: Asami Doi <asamidoi@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1269465}
The ARC boilerplate isn't needed any more and is being removed across
the codebase. Add a presubmit to avoid getting any more committed.
The presubmit change alone landed earlier and was reverted because it
wasn't ready. Now this lands along with other related changes.
Bug: 1468376
Change-Id: I9a7fdc66ed5daa1d623e3226fdf4e9004c071d5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727321
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1177895}
Currently, the link that is given is not recognized by apple default
terminal. Since it’s almost a given that any developer will have
internet access, using an online link seems acceptable. I selected the
public link given that Chrome is open source.
Change-Id: I565affd0fb24b6e8a007468781daa02c8f79dd11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3826240
Commit-Queue: Arthur Milchior <arthurmilchior@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034480}
The check is failing because it was written as if the content of the
file returned with f.NewContents() is a string. It is actually a list.
This CL fixes it.
The check never worked.
Bug: 798289
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Icfd216bbcaee78671878b65ac412b9063bd24509
Reviewed-on: https://chromium-review.googlesource.com/847372
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526452}
Add EarlGrey tests tryjobs to ios changes. We want to have EG
tests running for at least ios changes to reduce tree breakages.
Since the ios-simulator-full-configs has only 8 builders, iOS CLs might
experience extra landing time due to the tryjob size and limited capacity.
Bug: 782735
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8c00dbe1963893af19c37d7410e6d57c8d37ab5d
Reviewed-on: https://chromium-review.googlesource.com/759166
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Commit-Queue: Menglu Huang <huangml@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515214}
All TODOs in src/ios/ should follow the format TODO(crbug.com/######)
so add a PRESUBMIT.py check (and corresponding unit tests) for that.
BUG=677203
Change-Id: I360ad75c786f292d3a8ada8df519cb451c1c2828
Reviewed-on: https://chromium-review.googlesource.com/538613
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482589}
As we're adding more and more code outside of stc/ios/{web,chrome},
add a presubmit check enforcing "clang-format" for all of src/ios/
instead of limiting it to select sub-directories of src/ios/.
BUG=None
Review-Url: https://codereview.chromium.org/2665383002
Cr-Commit-Position: refs/heads/master@{#447541}
