Skip to content

Conversation

@DanilaFe
Copy link
Contributor

Clang-format is an automatic code formatting tool.

It would greatly ease the creation of consistent code: it follows exactly the rules in the .clang-format file. Style changes would be as easily applied as editing the .clang-format file and re-running the clang-format command:

clang-format src/lib_ccx/*.c src/lib_ccx/*.h src/ccextractor.c -i

The file that I'm proposing is a draft that can be easily customized by the maintainers to fit the code style they wanted for the project: I attempted to configure it such that it was similar to what the style appeared to be before.

clang-format is definitely a command line tool that works on macOS and Ubuntu, and to the best of my knowledge, it's also available as a Visual Studio plugin on Windows.

@cfsmp3
Copy link
Contributor

cfsmp3 commented Dec 15, 2016

Ah but maintainers don't want to customize themselves, we'd like to see the result of several options and then pick one :-)

@DanilaFe
Copy link
Contributor Author

That's possible too. I didn't run the tool because changing formatting will likely cause pointless conflicts.

@DanilaFe
Copy link
Contributor Author

How can I demonstrate the result of clang-format, other than by committing formatted files?

@cfsmp3
Copy link
Contributor

cfsmp3 commented Dec 15, 2016 via email

@DanilaFe
Copy link
Contributor Author

Here is a link to a zip file containing three different .clang-format files and the resulting ccextractor.c file.

The best-match style was written to try to match the current code style of the source code.
The linux and vs styles imitate the styles of the Linux Kernel and Visual Studio's formatting, respectively.

@cfsmp3
Copy link
Contributor

cfsmp3 commented Dec 23, 2016

I'm not going to merge because I'm quite torn here... I guess working on Python lately has made me switch my preference from 8 spaces to 4 for indents (which means no tab, which is not really C style).

But I liked seeing how this work, so it's something to revisit possibly during summer of code, after all the team has spent a few weeks on C :-)

@cfsmp3 cfsmp3 closed this Dec 23, 2016
hrideshmg pushed a commit to hrideshmg/ccextractor that referenced this pull request Mar 12, 2025
* Add test for variant output files in PR comment

Add a test that the info used to generate a PR comment handles variant
output files correctly. Regression tests that output one of the allowed
variants of the output file should be treated as correct in addition to
tests that output the original version of the output file.

* Refactor getting PR comment info into new function

Refactor the code for getting the info about a test run for use in a PR
comment into a separate function. This makes this function easier to
test instead of testing the entire process of creating a PR comment at
the same time.

* Fix variant output file handling in PR comments

Currently, when generating stats and a list of failed tests for PR
comments, tests are only marked as passed if they exactly matched the
original output file. This commit changes that by also treating a
regression test as passed if it is an acceptable variant of the output
file.

* Add test for invalid variant hash

Add another test to make sure that the change in
4e78b55aff5a6a3591059be2e64f91dedb978ef9 didn't accidentally cause the
PR comment handling to mark all tests as passing even if they result in
an invalid variant file.

* Rename get_info_about_test_for_pr_comment

nosetests appears to be treating get_info_about_test_for_pr_comment as a
test to run because it has 'test' in the name. That causes the test
suite to fail because it doesn't pass in the required argument.

This renames the function so that nosetests realizes that it shouldn't
treat it as a test.

* Move dataclasses to models.py

Move dataclasses used for representing info about PR comments to
models.py so that all classes for storing data are placed together.

Co-authored-by: Willem <github@canihavesome.coffee>
Co-authored-by: Shivam Kumar Jha <code@thealphadollar.me>
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.

2 participants