Skip to content

Clang format#1273

Draft
ZehMatt wants to merge 2 commits into
JonathanSalwan:dev-v1.0from
ZehMatt:clang-format
Draft

Clang format#1273
ZehMatt wants to merge 2 commits into
JonathanSalwan:dev-v1.0from
ZehMatt:clang-format

Conversation

@ZehMatt

@ZehMatt ZehMatt commented Aug 17, 2023

Copy link
Copy Markdown
Contributor

I've tried to minimize the diff but also didn't want to exactly match it to get somewhere in the middle of readability and consistency. I also enabled the option to auto comment the end of the namespace, this is a bit inconsistent at the moment and we should either turn that option off or remove the ones that don't fit, I personally think its best to let clang-format do the commenting so it won't be missed and it will be consistent across the board.

Probably need a few more files I should look into, clang-format can get quite notorious about tables.

@ZehMatt ZehMatt marked this pull request as draft August 17, 2023 13:13
@SweetVishnya

Copy link
Copy Markdown
Contributor

We could also add clang-format checks to CI

@ZehMatt

ZehMatt commented Aug 17, 2023

Copy link
Copy Markdown
Contributor Author

We could also add clang-format checks to CI

I could do that but its best to have all of the files formatted first

@JonathanSalwan

Copy link
Copy Markdown
Owner

Thanks for this MR,

I could do that but its best to have all of the files formatted first

Before updating all files, we should first agree on a format :). I will play with the example you provided and get back to you as soon as I can.

@ZehMatt

ZehMatt commented Aug 17, 2023

Copy link
Copy Markdown
Contributor Author

Before updating all files, we should first agree on a format :).

Definitely, that is also what I meant.

I will play with the example you provided and get back to you as soon as I can.

No rush here, thanks for looking into it.

@cnheitman

Copy link
Copy Markdown
Collaborator

This is a nice addition :)

@ZehMatt Could you modify the PR so it merges into dev-v1.0 instead of master?

@ZehMatt ZehMatt changed the base branch from master to dev-v1.0 August 25, 2023 14:24
@ZehMatt

ZehMatt commented Aug 25, 2023

Copy link
Copy Markdown
Contributor Author

This is a nice addition :)

@ZehMatt Could you modify the PR so it merges into dev-v1.0 instead of master?

Not sure why GitHub decided to pick master, I branched off dev-1.0 🤷‍♂️ , changed it.

@cnheitman

Copy link
Copy Markdown
Collaborator

Thanks, @ZehMatt !

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.

4 participants