Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGINGa451959e86bb: test: Add lint to prevent SIGNAL/SLOT connect style
rABCa451959e86bb: test: Add lint to prevent SIGNAL/SLOT connect style

Summary

Added new linter that enforces Qt5 SIGNAL/SLOT connection style.

Partial backport of Core PR13529 (adjusted to work with arcanist)
https://github.com/bitcoin/bitcoin/pull/13529/commits/3567b247f43decb6fc102d5b0989d1746fce0441

Test Plan

arc lint --everything

No Qt5 code violations should appear.

Change line 43 in qt/clientmodel.cpp from
connect(pollTimer, SIGNAL(timeout()), this, SLOT(updateTimer()));
to
connect(pollTimer, &QTimer::timeout, this, &ClientModel::updateTimer);

arc lint

Verify that two Qt5 SIGNAL/SLOT violation errors are reported, one for the SIGNAL and one for the SLOT.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR13529

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7899
Build 13808: Bitcoin ABC Buildbot (legacy)
Build 13807: arc lint + arc unit

Event Timeline

nakihito created this revision.Oct 29 2019, 20:30

Owners added a reviewer: Restricted Owners Package.Oct 29 2019, 20:30

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 29 2019, 20:30

nakihito planned changes to this revision.Oct 29 2019, 20:30

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito requested review of this revision.Oct 29 2019, 20:34

nakihito edited the summary of this revision. (Show Details)

nakihito added a parent revision: D4305: qt: Use new Qt5 connect syntax.

Adjusted a small comment in the lint-qt.sh script.

Build Bitcoin-ABC / Diffs / Diff Testing started.

nakihito edited the summary of this revision. (Show Details)Oct 29 2019, 20:39

Harbormaster completed remote builds in B7899: Diff 13753.Oct 29 2019, 20:41

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7900: Diff 13754.Oct 29 2019, 20:52

nakihito edited the summary of this revision. (Show Details)Oct 29 2019, 21:05

Please update the diff dependencies according to the parent being splitted.

The script is no longer used since the regex is part of the PHP script. You should remove it and update the comments accordingly.

As discussed offline, is it safe to only check for SIGNAL or SLOT only in order to avoid having 2 errors for the same connect() call ? Is there some case where one could appear and not the other ?

This revision now requires changes to proceed.Oct 30 2019, 10:42

Removed unnecessary script.

nakihito planned changes to this revision.Oct 30 2019, 19:02

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7914: Diff 13792.Oct 30 2019, 20:12

nakihito removed a parent revision: D4305: qt: Use new Qt5 connect syntax.Oct 31 2019, 20:48

nakihito edited the summary of this revision. (Show Details)

Rebased.

In D4306#104236, @Fabien wrote:

Please update the diff dependencies according to the parent being splitted.

The script is no longer used since the regex is part of the PHP script. You should remove it and update the comments accordingly.

As discussed offline, is it safe to only check for SIGNAL or SLOT only in order to avoid having 2 errors for the same connect() call ? Is there some case where one could appear and not the other ?

Script removed and dependency removed (all changes have been landed).
Unfortunately, it is not guaranteed that one will occur with the other. See the following:
SLOT only: https://github.com/bitcoin/bitcoin/pull/13529/commits/f78558f1e39198779bdb17e2b0e256fb99ad4b28#diff-0db7dd184df07a48c307ccc182021a68L959
SLOT only: https://github.com/bitcoin/bitcoin/pull/13529/commits/f78558f1e39198779bdb17e2b0e256fb99ad4b28#diff-bb3be7e8e1a771895eedfe9802528225L21
SIGNAL only: https://github.com/bitcoin/bitcoin/pull/13529/commits/f78558f1e39198779bdb17e2b0e256fb99ad4b28#diff-1bd3480f178e8bf9a556293cc3c6103eL216