Skip to content

Conversation

@Davoodeh
Copy link

@Davoodeh Davoodeh commented Mar 30, 2025

No changes but style.

Ideally, I suppose every editor is configured to be format-on-save. This change is a vital change for such configurations before people of those type can contribute. I may also make an .editorconfig file compatible with this.

This serves as an example, discussions are to have about what format to use (indent is another candidate, having tested both, clang-format --style=gnu really takes the cake with the default no-tabs and formatting macros and sometimes comments).

@fzerorubigd
Copy link
Collaborator

I wanted to do this a long time ago. The problem was it changes the files in a way that tracking the history can be a little problematic if you just go for a blame :) but I guess it is ok, and if it should be done, lets do it and get on with it,

@fzerorubigd fzerorubigd merged commit 50b47a4 into persiancal:master Apr 5, 2025
2 checks passed
@Davoodeh
Copy link
Author

Davoodeh commented Apr 5, 2025

Okay then, so clang default is the way to go then?!

Since literally "I am to blame," I'll just format everything related in the project and make another PR.

Thank you, it's a pain to develop on not-auto-formatted codebases.

Davoodeh added a commit to Davoodeh/jcal_c that referenced this pull request Apr 25, 2025
This action, triggered on push or pull, fails if clang-format fails.
This addresses the comment referenced by [1] on persiancal#28 (formatting all the
sources following the minimal path examples on persiancal#23).

Note that the "sources" directory is hardcoded on the action file
pushed. Assuming the change of source directory is a fundamental change
with a lot of considerations, no documentations were added to mention
that the value used in this file must be synchronized with the directory
name on the tree. Suppose this comment suffices.

[1]: persiancal#28 (comment)
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