-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[pretty] Add example sources to pretty. #430
Merged
+81
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # | ||
| # Copyright (c) 2020 Project CHIP Authors | ||
| # Copyright (c) 2014-2018 Nest Labs, Inc. | ||
| # Copyright (c) 2018 Google LLC | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
|
|
||
| # | ||
| # Description: | ||
| # This file is the GNU automake template for the CHIP SDK. | ||
| # | ||
|
|
||
| include $(abs_top_nlbuild_autotools_dir)/automake/pre.am | ||
|
|
||
| SUBDIRS = \ | ||
| $(NULL) | ||
|
|
||
| EXTRA_DIST = \ | ||
| $(NULL) | ||
|
|
||
| PRETTY_FILES = \ | ||
| lock-app/efr32/include/AppEvent.h \ | ||
| lock-app/efr32/include/LEDWidget.h \ | ||
| lock-app/efr32/include/AppTask.h \ | ||
| lock-app/efr32/include/CHIPProjectConfig.h \ | ||
| lock-app/efr32/include/sample_qr_code.h \ | ||
| lock-app/efr32/include/BoltLockManager.h \ | ||
| lock-app/efr32/include/AppConfig.h \ | ||
| lock-app/efr32/include/ble-configuration.h \ | ||
| lock-app/efr32/include/init_board.h \ | ||
| lock-app/efr32/include/init_mcu.h \ | ||
| lock-app/efr32/include/hal-config-app-common.h \ | ||
| lock-app/efr32/include/init_lcd.h \ | ||
| lock-app/efr32/include/FreeRTOSConfig.h \ | ||
| lock-app/efr32/include/hal-config.h \ | ||
| lock-app/efr32/include/board_features.h \ | ||
| lock-app/efr32/include/mbedtls-config.h \ | ||
| lock-app/efr32/include/ButtonHandler.h \ | ||
| lock-app/efr32/src/main.cpp \ | ||
| lock-app/efr32/src/display/init_lcd.c \ | ||
| lock-app/efr32/src/display/sample_qr_code.c \ | ||
| lock-app/efr32/src/AppTask.cpp \ | ||
| lock-app/efr32/src/init_mcu.c \ | ||
| lock-app/efr32/src/init_board.c \ | ||
| lock-app/efr32/src/LEDWidget.cpp \ | ||
| lock-app/efr32/src/ButtonHandler.cpp \ | ||
| lock-app/nrf5/main/BoltLockManager.cpp \ | ||
| lock-app/nrf5/main/include/OpenThreadConfig.h \ | ||
| lock-app/nrf5/main/include/nrf_log_ctrl_internal.h \ | ||
| lock-app/nrf5/main/include/AppEvent.h \ | ||
| lock-app/nrf5/main/include/LEDWidget.h \ | ||
| lock-app/nrf5/main/include/AppTask.h \ | ||
| lock-app/nrf5/main/include/CHIPProjectConfig.h \ | ||
| lock-app/nrf5/main/include/freertos_tasks_c_additions.h \ | ||
| lock-app/nrf5/main/include/BoltLockManager.h \ | ||
| lock-app/nrf5/main/include/FreeRTOSConfig.h \ | ||
| lock-app/nrf5/main/include/app_config.h \ | ||
| lock-app/nrf5/main/main.cpp \ | ||
| lock-app/nrf5/main/AppTask.cpp \ | ||
| lock-app/nrf5/main/LEDWidget.cpp \ | ||
| lock-app/nrf5/main/ldscripts/chip-nrf52840-lock-example.ld \ | ||
| lock-app/nrf5/main/support/AltPrintf.c \ | ||
| lock-app/nrf5/main/support/FreeRTOSNewlibLockSupport.c \ | ||
| lock-app/nrf5/main/support/nRF5Sbrk.c \ | ||
| lock-app/nrf5/main/support/CXXExceptionStubs.cpp \ | ||
| $(NULL) | ||
|
|
||
| include $(abs_top_nlbuild_autotools_dir)/automake/post.am | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@turon First time I'm digging into pretty, so sorry for the dumb question... is there a way to make this such that every developer doesn't have to remember to do this for every file added? (Re-inforces why I prefer restyled).
Maybe we can use restyled.io locally instead of make pretty? @rwalker-apple thoughts? That way we have stuff that's at least inline, and doesn't require this manual addition of stuff...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woody-apple I'm a big proponent of simplifying the developer experience and resonate with the goals you mention; unfortunately, I don't know of a simpler way to do it with autotools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@turon would you be against me moving make pretty to run this? https://github.com/restyled-io/restyler
It does exactly what our repository is doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woody-apple I don't have a problem with it. @gerickson?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as it can run offline and doesn't explicitly require a Docker container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gerickson Why is offline and docker required for active development?
Trying to understand the resistance here to matching to what the repository does? Seems like a benefit to use a general system that we've deployed here rather than one that requires each developer to understand which file to edit to make sure make-pretty works.
Note: You only need to grab the container once with Docker, so technically doesn't require online.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In terms of offline, I'd like to ensure that, as a developer, if I happen to be on a plane or somewhere where I can't consistently ping out to remote services that I can do everything I need to do to land commits locally where the push to remote is a final step I take when I land or get connectivity back. If style compliance is part of landing commits locally with confidence that I've passed all quality gates, then the tooling to do that should be local.
I may not understand restyled.io well at this point; however, my understanding is that the service is running remote tooling to do what clang-format formerly did locally. But, like travis-ci and others, it may just be triggering clang-format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restyled.io just uses clang-format, does exactly the same thing that make-pretty tries to do, but does so globally on files rather than using a developer supplied list of files to make-pretty. It also uses shell check for others, as well as markdown cleanup using 'pretty-markdown'.
I agree we need the tooling to work on the developer's machine & it's true that you need to download the docker image once, but then it's offline forever. That doesn't seem like a burden personally. Same goes for builds & VSCode development now, if you want builds to run like travis, you need to download the container once, and then you're good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a "format entire tree" as a script in integrations/ would be my preference, but is a low priority given how awesome restyled has been so far
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handling the prettifying of the tree here: #432
Another PR to come later with some changes to make this easy for anyone to run.