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

Make code compliant with editorconfig and cpplint #32

Merged
merged 41 commits into from
Feb 7, 2023

Conversation

ptahmose
Copy link
Contributor

@ptahmose ptahmose commented Feb 1, 2023

Description

  • added an .editorconfig file
  • formatted all code with those settings

Fixes #31

About every file has been touched in the PR, so it is probably impossible to review it. However - the hope is that this is the last time that we have a change like that (i.e. a massive PR due to formatting changes).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

built locally and ran tests

Checklist:

  • I followed the Contributing Guidelines.
  • I did a self-review.
  • I commented my code, particularly in hard-to-understand areas.
  • I updated the documentation.
  • I updated the version of libCZI following Versioning of libCZI depending on the type of change
    • Bug fix -> PATCH
    • New feature -> MINOR
    • Breaking change -> MAJOR
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Merging #32 (7c9881f) into main (078cf06) will decrease coverage by 0.01%.
The diff coverage is 53.13%.

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
- Coverage   47.27%   47.27%   -0.01%     
==========================================
  Files          77       77              
  Lines       15749    15756       +7     
==========================================
+ Hits         7445     7448       +3     
- Misses       8304     8308       +4     
Flag Coverage Δ
windows-latest 47.27% <53.13%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
Src/libCZI/CziMetadataDocumentInfo2.cpp 50.20% <ø> (ø)
Src/libCZI/CziReaderWriter.cpp 79.88% <ø> (ø)
Src/libCZI/CziStructs.cpp 85.71% <ø> (ø)
Src/libCZI/CziSubBlockDirectory.cpp 79.88% <ø> (ø)
Src/libCZI/CziSubBlockDirectory.h 100.00% <ø> (ø)
Src/libCZI/CziUtils.cpp 62.10% <ø> (ø)
Src/libCZI/CziUtils.h 80.00% <ø> (ø)
Src/libCZI/CziWriter.cpp 84.82% <ø> (ø)
Src/libCZI/CziWriter.h 65.47% <ø> (ø)
Src/libCZI/DimCoordinate.cpp 66.93% <ø> (ø)
... and 91 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@FelixS90 FelixS90 left a comment

Choose a reason for hiding this comment

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

I checked the content with "Hide Whitespace", which left over 8 files to review.

image

As discussed offline, I will try to bring this into a GitHub Action to avoid regressions in the future.

Options:

  1. Mega-Linter using editorconfig-checker under the hood
  2. EditorConfig-Action

.editorconfig Show resolved Hide resolved
Src/libCZI/CziUtils.cpp Show resolved Hide resolved
Src/libCZI/CziUtils.cpp Show resolved Hide resolved
.editorconfig Show resolved Hide resolved
.editorconfig Show resolved Hide resolved
.editorconfig Show resolved Hide resolved
@ptahmose
Copy link
Contributor Author

ptahmose commented Feb 2, 2023

I checked the content with "Hide Whitespace", which left over 8 files to review.

Thanks for this tip, wasn't aware of this feature. Nice!

@FelixS90 FelixS90 added the cla Contributor License Agreement sent to Admin label Feb 6, 2023
@FelixS90 FelixS90 mentioned this pull request Feb 7, 2023
CPPLINT.cfg Show resolved Hide resolved
Copy link
Collaborator

@FelixS90 FelixS90 left a comment

Choose a reason for hiding this comment

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

As with the first review, I checked all changed files that do not only have white space changes.

I did not check Cpplint related changes for correctness - trusting cpplint itself and tests.

Feel free to resolve all remaining conversations if you are OK with the changes I introduced on top of this PR.

@FelixS90 FelixS90 changed the title Untabify and add editorconfig Make code compliant with editorconfig and cpplint Feb 7, 2023
@ptahmose ptahmose merged commit a3b4a97 into ZEISS:main Feb 7, 2023
@FelixS90 FelixS90 mentioned this pull request Feb 7, 2023
11 tasks
@ptahmose ptahmose deleted the untabify_and_add_editorconfig branch June 8, 2023 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla Contributor License Agreement sent to Admin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add "EditorConfig"-configuration to project, and tidy up the formatting in all files
3 participants