search

FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
367 stars 115 forks source link

ENH: Use findent to indent all .F90 files #366

Open Patol75 opened 1 year ago

Patol75 commented 1 year ago

I have run into findent today, and I tried it on all .F90 source files within the Fluidity tree. The result lies within the branch associated with the present PR. Provided that CI returns green, would you say that such a change is desirable? Unfortunately, findent is not available as a pre-commit hook, but perhaps it will be in the future.

gnikit commented 1 year ago

I will see if I can make a pre-commit hook today. FYI fprettify is another option although it will do more than just indentation.

cianwilson commented 1 year ago

While I see that this change is aesthetically pleasing I'm curious what its benefits are beyond that.

The major downside appears to be that it will almost certainly make every branch conflict on merge. I'm already finding that in my branches with the existing hooks. So I'm curious what the upside is beyond aesthetics.

Thanks!

gnikit commented 1 year ago

While I see that this change is aesthetically pleasing I'm curious what its benefits are beyond that.

findent can do some refactoring and Fixed -> Free form conversion, but I personally wouldn't use it for that. Fluidity is stable, so "if it ain't broke don't fix it" seems like a solid strategy.

The major downside appears to be that it will almost certainly make every branch conflict on merge.

Specifically for findent that shouldn't happen. Assuming we use findent for indentation and nothing else, the only diff from the base should be whitespaces, no new lines, no added code, which a simple rebase is able to resolve.

The only nontrivial downside of formatting the entire codebase would be that navigating git file history would be slightly more annoying. I personally don't mind since I use VS Code + GitLens which is a GUI, but I understand that the majority of the other devs are more terminal-driven in their development.

In general, aesthetic changes such as this, are by definition subjective and reinforced by personal preferences so I'm okay with whatever you decide to do with this PR.