Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tidy and format to make rules #47

Merged

Conversation

rwalker-apple
Copy link
Contributor

@rwalker-apple rwalker-apple commented Mar 12, 2020

Problem

clang-tidy and clang-format are not part of developer workflow

Changes

  • add clang-tidy and clang-format to rules for C files in the tree
  • allow overriding tidy and format for subtrees

fixes #2

Copy link
Contributor

@gerickson gerickson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all seems fine; however, I believe @turon is working a PR for autotools for the build and, with that, it would be great to see this as an implementation behind the 'make pretty' and 'make pretty-check' targets that can be updated, enhanced, evolved, independently of the high-level, user-facing targets.

@woody-apple
Copy link
Contributor

@hawk248 or @BroderickCarlin Thoughts?

@gerickson
Copy link
Contributor

Can we split this up into two PRs: one that adds the functionality and another that submits the changes based on running that functionality?

@rwalker-apple
Copy link
Contributor Author

Can we split this up into two PRs: one that adds the functionality and another that submits the changes based on running that functionality?

Sure, but can you help me understand the value of such?

To accomplish such, I'd submit the re-formatted source files first, then add the tooling in a later PR because the tooling is currently part of continuous integration. Would that work for you?

@woody-apple woody-apple requested a review from gerickson March 12, 2020 23:13
@gerickson
Copy link
Contributor

Can we split this up into two PRs: one that adds the functionality and another that submits the changes based on running that functionality?

Sure, but can you help me understand the value of such?

To accomplish such, I'd submit the re-formatted source files first, then add the tooling in a later PR because the tooling is currently part of continuous integration. Would that work for you?

I may be a bit confused here; however, when I'd chatted with @woody-apple, he'd suggested that the approach for style compliance was warn-and-flag-and-make-the-user-resubmit manually rather than automatically reformatting and folding those into a PR. Consequently, I'd expect a PR that adds the support for running style check or style reformat targets and then another PR that is the results of someone running those targets on the tree.

In terms of value, it just cleanly separately concerns into isolated chunks of work that are independent of one another. I think that's generally good hygiene for the project in the long-term as a policy for pull requests.

@rwalker-apple rwalker-apple force-pushed the Add-tidy-and-format-to-make branch from b20f862 to 6f43121 Compare March 13, 2020 18:56
@rwalker-apple
Copy link
Contributor Author

this is now broken up such that a rebase (which I will do once #53 lands) will remove the source changes

@rwalker-apple rwalker-apple force-pushed the Add-tidy-and-format-to-make branch from bdcc707 to fc6095a Compare March 13, 2020 19:05
@woody-apple
Copy link
Contributor

@hawk248 ?

@woody-apple woody-apple merged commit 0f5f950 into project-chip:master Mar 13, 2020
@hawk248
Copy link

hawk248 commented Mar 13, 2020

No objections ... use it where appropriate.

@rwalker-apple rwalker-apple deleted the Add-tidy-and-format-to-make branch March 13, 2020 20:11
doublemis1 referenced this pull request in doublemis1/connectedhomeip Jul 26, 2021
* Add simple xml rpc server to chip device controller

* Fix RPC Server address

* Remove message lowercasing on rpc

* Remove redundant code

* Place all rpc, csg changes at the bottom of the file

* Add todo comment

* Add extra newlines

* Move the csg additions to above the main method, cannot forward declare the methods

* Open qr_code_parse to dict api for RPC server

* Added development guide

* Added Setup and Testing guide for Real and Virtual BLE Accessories

* Updating Linux Lighting app docker file

* demarcator added in setup_payload, new line at the end

* Implemented Resolve method.

* Code review comments resolved.

* Implementation of ble-scan method with returning discovered devices (#18)

* Implemented ble_scan with peripheral list return.

* Update chip-device-ctrl.py

* Implementation of ZCL AddNetwork and EnableNetwork methods (#23)

RPC methods for Resolve and Zcl - AddNetwork and EnableNetwork implemented.

* Add the test_client sample program

* Updated Readme

* Added new lines

* Updated comments and readme

* Updated README

* Updated readme

* Update chip-device-ctrl.py

* Update chip-device-ctrl.py

* Csg/feature/get pase data (#30)

* Remove message lowercasing on rpc

* Remove redundant code

* Move the csg additions to above the main method, cannot forward declare the methods

* Remove message lowercasing on rpc

* Add debug prints and extra getpase command

* Add get pase session

* Initial PASE data fetch skeleton

* Add separate csg folder to hold constants and utils

* Add macros

* GetPaseData should return a yaml str.

* Add #defines, refactor into csg utilities, deserialize yaml

* Fix linter, comments, add copyright

* added get_pase_data RPC

* refactor methods and variables

* Add PASE Response parameters

* Debuggin additions

* Remove debugging additions

* Fix RPC to allow none values

* Remove pase from BLE call

* Added response, pake 1,2,3 parameters

* Remove merge artifact

* Remove unnecessary edits

* Add debug messages

* Remove debug messages

* Add todo

Co-authored-by: Mikael H. Moeller <mikaelhm@apple.com>

* Get fabric ID (#35)

* Get fabric ID

* Added Introspection for RPC methods (#39)

* Added Introspection for RPC methods

* Add pin code parsing

* Update setup payload return

* Remove extra spaces, extra newline

* Rename to pin_code_parse

* Add ble close

* Add build_sdk only script

* Remove extra space from argument assignment

* Add new line at the end

* Refactor test client build scripts

* Add new line

* Added resolve instruction and dockerfile fixes

* Implement a generic cluster RPC call

* Implement a generic cluster RPC call

* removing unwanted comment

* Addressing PR comments

* Addressing PR review comments

* correcting typing errors

* Update chip-device-ctrl.py

* Undo merge changes

Co-authored-by: Xiaolei Zhou <xiole@amazon.com>
Co-authored-by: SaileeG <SaileeG@github.com>
Co-authored-by: Nirav Shah <nirav_shah2@cable.comcast.com>
Co-authored-by: Nirav Shah <shah.niravk@gmail.com>
Co-authored-by: Mikael H. Moeller <mikaelhm@apple.com>
Co-authored-by: SaileeG <skorgaonkar@google.com>
Co-authored-by: SaileeG <69006439+SaileeG@users.noreply.github.com>
lpbeliveau-silabs pushed a commit to lpbeliveau-silabs/connectedhomeip that referenced this pull request Sep 12, 2022
…from the new target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
mkardous-silabs referenced this pull request in SiliconLabs/watt-sandbox Oct 6, 2022
… target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
nipatel-silabs pushed a commit to nipatel-silabs/connectedhomeip that referenced this pull request Oct 19, 2022
…from the new target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
mkardous-silabs referenced this pull request in mkardous-silabs/connectedhomeip Oct 24, 2022
… target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
mkardous-silabs referenced this pull request in mkardous-silabs/connectedhomeip Nov 2, 2022
… target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
shgutte pushed a commit to shgutte/connectedhomeip that referenced this pull request Oct 5, 2023
…from the new target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
shgutte pushed a commit to shgutte/connectedhomeip that referenced this pull request Jan 11, 2024
…from the new target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
mykrupp pushed a commit to mykrupp/connectedhomeip that referenced this pull request Jul 18, 2024
…from the new target name

Merge in WMN_TOOLS/matter from fix_chiptool_path to silabs

Squashed commit of the following:

commit 2e67137849d78bcef91d10629774f14aaf61734a
Author: Junior Martinez <junior.martinez@silabs.com>
Date:   Tue Aug 16 16:58:37 2022 -0400

    Fix path by adding the missing -clang- from the new target name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run clang-tidy locally
6 participants