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

Disable clang-format from super-linter #24

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

razvand
Copy link
Contributor

@razvand razvand commented Nov 1, 2022

Disable clang-format from super-linter

To see how this works and the amounts of seemingly unconfigurable issues that appear:

  1. Install using pip. This installs a newer 14 version.

  2. Dump the current configuration to get all (?) the configuration options:

    clang-format -style=llvm -dump-config > current_clang-format
    
  3. See configuration options.

  4. To emulate the super-linter behavior, run:

    clang-format --dry-run -Werror chapters/data/lab/support/alloc-size/alloc_size.c
    

There are -Wclang-format-violation warnings that we weren't able to get rid of. We didn't find the configuration options. Some situations:

  • Having a Tab between the name of a macro and its value. clang-format requires to use a certain number of space. We didn't find any option to configure the use of tabs.
  • clang-format is strict to the use of spaces around arithmetic operators. For example, using int a = b-1; triggers a warning / error to have it written as int a = b - 1;. This seems a bit too much and should be configurable. But again, we didn't find an option to disable this.

A rather complete configuration file is the one used by cpp_weekly.

@razvand razvand added the enhancement New feature or request label Nov 1, 2022
@razvand razvand requested a review from teodutu November 1, 2022 17:14
@razvand razvand self-assigned this Nov 1, 2022
Copy link
Member

@teodutu teodutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You made this branch from Data_lab, so you carried over the changes to the data lab to this PR.
  2. Isn't this too drastic? I know our coding style isn't too compatible with the clang standard, but are they that different that it requires disabling clang-format altogether?

clang-format is very pedantic about coding style. This commit disables
is from super-linter (in the `super-linter.yml` file) and removes the
configuration file `.clang-format`.

Signed-off-by: Razvan Deaconescu <razvan.deaconescu@cs.pub.ro>
@razvand
Copy link
Contributor Author

razvand commented Nov 1, 2022

  1. You made this branch from Data_lab, so you carried over the changes to the data lab to this PR.
  2. Isn't this too drastic? I know our coding style isn't too compatible with the clang standard, but are they that different that it requires disabling clang-format altogether?
  1. Fixed.
  2. See my update to the PR message.

Copy link
Member

@teodutu teodutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the additional details you provided, I now believe you're right to remove clang-format. cpplint should suffice for basic errors such as trailing whitespaces.

@lizababu lizababu self-requested a review November 10, 2022 11:09
@razvand razvand merged commit 4b87f7e into open-education-hub:master Nov 10, 2022
@razvand razvand deleted the disable-clang-format branch November 10, 2022 11:10
@razvand razvand mentioned this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants