Skip to content

Initial basic editorconfig support#3212

Merged
WardF merged 5 commits intoUnidata:mainfrom
seanm:editorconfig1
Dec 2, 2025
Merged

Initial basic editorconfig support#3212
WardF merged 5 commits intoUnidata:mainfrom
seanm:editorconfig1

Conversation

@seanm
Copy link
Contributor

@seanm seanm commented Nov 16, 2025

If you've never heard of it: https://editorconfig.org/

Specify only:
- unix line endings
- insert final newlines
- UTF-8 encoding
There were only 4 files that didn't use UNIX line endings, found with the `editorconfig-checker` tool.  Changed with:

perl -pi -e 's/\r$//' cmake/modules/windows/FindHDF5.cmake docs/obsolete/fan_utils.html docs/old/netcdf-internal.texi docs/static-pages/orgs.html
Found with `editorconfig-checker` checker tool. Fixed manually.
@DennisHeimbigner
Copy link
Collaborator

I don't know Ward and I have noticeably different coding styles.
What do you think, Ward?

@seanm
Copy link
Contributor Author

seanm commented Nov 16, 2025

I don't know Ward and I have noticeably different coding styles. What do you think, Ward?

There are no changes to coding styles here, in this initial PR.

If we choose, it could be used to bring more uniform indentation though. I notice a lot of the codebase has screwed up indentation...

@seanm
Copy link
Contributor Author

seanm commented Nov 23, 2025

I don't know Ward and I have noticeably different coding styles. What do you think, Ward?

Friendly ping. @WardF @DennisHeimbigner

@mannreis
Copy link
Contributor

I think any effort on code formatting is very valuable for external contributors! I'd even support checks via pre commit hook

@dopplershift
Copy link
Member

I don't know Ward and I have noticeably different coding styles. What do you think, Ward?

$0.02 from the peanut gallery: Multiple styles in a single code-base is a bug, not a feature. Long-term sustainability for the project is helped by making it as easy as possible to on-board new contributors.

@WardF
Copy link
Member

WardF commented Dec 2, 2025

Taking a look now. I was unfamiliar with this, and was prioritizing other tasks. Looking now.

@WardF WardF merged commit dd22155 into Unidata:main Dec 2, 2025
103 checks passed
@seanm
Copy link
Contributor Author

seanm commented Dec 2, 2025

@WardF great, thanks! With this basic initial pass accepted, the next pass I'd propose is enabling the setting to delete all trailing whitespace... sound ok?

@captainkirk99
Copy link

THis is a great idea. Different code styles are not great...

@DennisHeimbigner
Copy link
Collaborator

Give them an inch...

@seanm
Copy link
Contributor Author

seanm commented Dec 2, 2025

I gave it a quick try, and there is in fact a lot of trailing whitespace. This is easily remedied with a perl one liner like find . -name '*.c' -exec perl -pi -e 's/[ \t]+$//' {} +.

I could make such an MR, but it's of course super tedious to review. Could also be a sneaky way to insert a vulnerability. :) Perhaps @WardF or @DennisHeimbigner you'd prefer to do so yourself?

@seanm
Copy link
Contributor Author

seanm commented Dec 2, 2025

May as well just show the result: eefa097

@dopplershift
Copy link
Member

Give them an inch...

Funny, that's what I usually say about the Unidata dev team 😆

@captainkirk99
Copy link

I think any objective observer of the code will conclude that my code has perfect style, and I recommend it be used as the template for correct style, not just in the netcdf-c and netcdf-fortran projects, but everywhere.

However, in case there are (incredibly!) any other opinions, I'm willing to work to any common style, even one noticeably worse than my own.

@seanm I think you have found the correct method to move forward: pick one improvement at a time. Removing whitespace at the end of the lines is a great starting point. I applaud your efforts and urge you to dig in for the long haul on this one.

@seanm
Copy link
Contributor Author

seanm commented Dec 4, 2025

I think any objective observer of the code will conclude that my code has perfect style, and I recommend it be used as the template for correct style, not just in the netcdf-c and netcdf-fortran projects, but everywhere.

rotfl :)

@seanm I think you have found the correct method to move forward: pick one improvement at a time. Removing whitespace at the end of the lines is a great starting point.

Yeah, it seems like something that's hard to argue has any benefit, so easier to get agreement on removing.

And, as I said, can be done with a perl one-liner. Ideally by one of the core maintainers so as to allow skipping any human tediously reviewing an external contribution.

@captainkirk99
Copy link

Currently there is only one maintainer of this repo, @ward. All others working on netcdf-c are outside of Unidata and don't have the ability to apply these changes.

@WardF
Copy link
Member

WardF commented Dec 5, 2025

@WardF for what it's worth, and yeah. I'm working as quick as I can, but the main issue right now is to get through the other meatier ones that have lingered during holiday/grant writing/AGU season. I will see about getting the others in ASAP, but from a priority standpoint, we're a single threaded process and right this second I'm wiring in HDF5 2.0.0 stuff into our local test infrastructure, which will feed into our automated test infrastructure, and no doubt work perfectly and not result in any additional overhead or tasks ;).

That said, even small changes have an overhead that takes a bit of time.

Honestly though, I'd rather have all this work to do than not have anything to do. As @captainkirk99 no doubt will attest, working on netCDF is extremely rewarding, and I am grateful for all of the contributions and effort people put in. The work you all collectively do deserves my full attention, and also the work others do deserves my full attention so as to not foul up the work you are trying to do simply because I rushed and accepted something without really thinking it through :). I'm trying to clear the backlog before EOY. Let's see what happens.

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.

6 participants