-
Notifications
You must be signed in to change notification settings - Fork 712
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
Introduce clang-format as part of CI using Github Actions #2017
Conversation
f0e64fd
to
df66dd2
Compare
The build fails with missing api_token. I believe I've created GH_TOKEN with the access rights for pushing to OpenSC/Nightly only. Maybe you need to create an other one for pushing to |
Thanks for pointer. I am testing the actions in my fork. Now, I generated my own GH_TOKEN in the fork's PR so lets see how it will work. I will probably push some testing code to be able to check how does it actually work with real code. |
@frankmorgner the above is already tested with one of the commits from my Fuzz branch, successfully reporting some diffs through github actions (as I am not that precise with formatting ;) ). The travis does not work because there is probably some old version of clang in the osx builder (does not know |
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.
great, if the formatting matches the majority of the code base, we're good to go
I think we're good for merging, right? |
Not yet. I wanted to review some of more obscure options of the code formatter, but did not get to do that yet. Please, keep this one open before we will finalize the format in #1641. |
d109a0a
to
515dcc3
Compare
b8996cb
to
a4d3bb5
Compare
What's the status of this pull request? |
It is awfully outdated. But I did not get much feedback on my comments in #1641 during last year so I did not push that, especially when we were in need of release. It is now out so we can try to focus to this agian. I saw @dengert to test this, coming to some issues, but already using something in his PRs. I will get it up to date and probably try the Github action version as the travis is getting awfully slow recently. |
7dae41d
to
974da68
Compare
This allows for voluntary formatting with clang-format (or IDE tools like clang-format plugin for VS Code). Once a suitable formatting configuration is found, automatic formatting can be configured Change-Id: I9021a98fe90c5ef9f21ce7459fdf05fa645fb56c
Usage: git config blame.ignoreRevsFile .ignoreRevsFile
fbe71f2
to
b901c42
Compare
Apparently, this PR is not in a condition to be merged as it is, but I would ask if at least the single .clang-format file from this package could not be merged into the master. Thank you. |
Yeah. I spent already too much time reformatting the code, while trying to understand some of the options and their behavior. I moved the non-problematic changes into separate PR #2977. Some less disruptive changes on top of them are in https://github.com/Jakuje/OpenSC/tree/clang-format for preview how it tries to format some larger code changes (the leftovers marked as clang format now are mostly the changes that I do not like or missed). |
Testing of #1641