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

reorder headers and break into blocks in src/app #685

Merged
merged 2 commits into from
May 18, 2020

Conversation

rwalker-apple
Copy link
Contributor

@rwalker-apple rwalker-apple commented May 13, 2020

Problem

includes aren't sorted

Summary of Changes

  • sort includes
  • break into blocks for things that shouldn't be sorted (e.g. gen-* headers aren't stand-alone, need to have zcl stuff before them)
  • make more headers stand alone

working on #116, part of #683

@rwalker-apple
Copy link
Contributor Author

@wehale @tecimovic would you have a look?

the changes are grouping headers by sorting blocks, where necessary to get the code compiling with sorted include directives

@tecimovic
Copy link
Contributor

@rwalker-apple : the header sorting and cleanup is a worthy exercise, but the entire src/app is nowhere near the state where this really matters at this point. I think we'll break down the headers some more, separate the APIs into upward looking ones (callbacks) and downward looking ones (API propers), and most of all, deal with the includes for the generated stuff, which should in the final solution not be #included in, but linked in.

@tecimovic
Copy link
Contributor

@rwalker-apple : one more thing: I see your diffs are operating on a directory structure as it was before the PR that I'm having in the queue now.

There will be an annoying merge conflict either way, so I think at this point, getting a PR in that touches files in the old directory structure, will just cause a nightmare of a merge.

@rwalker-apple
Copy link
Contributor Author

@rwalker-apple : the header sorting and cleanup is a worthy exercise, but the entire src/app is nowhere near the state where this really matters at this point. I think we'll break down the headers some more, separate the APIs into upward looking ones (callbacks) and downward looking ones (API propers), and most of all, deal with the includes for the generated stuff, which should in the final solution not be #included in, but linked in.

this is to forestall future issues when auto-formatting starts enforcing this

@rwalker-apple
Copy link
Contributor Author

@BroderickCarlin @woody-apple ?

@rwalker-apple
Copy link
Contributor Author

@rwalker-apple : one more thing: I see your diffs are operating on a directory structure as it was before the PR that I'm having in the queue now.

There will be an annoying merge conflict either way, so I think at this point, getting a PR in that touches files in the old directory structure, will just cause a nightmare of a merge.

I have a lot of experience fixing up merges, can help.

@rwalker-apple
Copy link
Contributor Author

@hawk248 @saurabhst @jelderton ?

@rwalker-apple
Copy link
Contributor Author

@BroderickCarlin

@anush-apple anush-apple merged commit 7b667c4 into project-chip:master May 18, 2020
@rwalker-apple rwalker-apple deleted the sort-includes-app branch May 19, 2020 17:07
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.

8 participants