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

draft: experimental code formatting #2199

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RuiwenTang
Copy link
Collaborator

As mentioned in #1519
We need an automated code formatting tool and consistent code style
This PR shows how to use clang-format to define a coding style and use .git-blame-ignore-revs to ignore the format commit from normal gitblame.
github can apply this ignore list automatically. For local repository, need run the following command:

git config blame.ignoreRevsFile .git-blame-ignore-revs

image

@RuiwenTang
Copy link
Collaborator Author

@hermet Do you think this method is acceptable?

@hermet hermet added infrastructure Dev infrastructure refactoring Code refactoring / Exceptional handles labels Apr 18, 2024
@hermet
Copy link
Member

hermet commented Apr 18, 2024

@hermet Do you think this method is acceptable?

@RuiwenTang Firstly, I'm welcome to this. Here are two major points we need to make a consensus:

  • When to apply? This change will bring huge code modifications that may be difficult for maintainers of other systems to integrate with the newer versions of ThorVG. That's why I've considered the change point is at 1.0 release.
  • What coding convention standard do we prefer? There are a bunch of different coding standard by the well-known organizations/companies and ThorVG has no connection to Google so far.

@RuiwenTang
Copy link
Collaborator Author

RuiwenTang commented Apr 18, 2024

@hermet Do you think this method is acceptable?

@RuiwenTang Firstly, I'm welcome to this. Here are two major points we need to make a consensus:

  • When to apply? This change will bring huge code modifications that may be difficult for maintainers of other systems to integrate with the newer versions of ThorVG. That's why I've considered the change point is at 1.0 release.
  • What coding convention standard do we prefer? There are a bunch of different coding standard by the well-known organizations/companies and ThorVG has no connection to Google so far.

I think it all depends on you.
As the number of code and contributors increases, global formatting becomes increasingly expensive. Also, I think the current coding style might cause confusion when others are first contributing to ThorVG

We can discuss the details of coding style together, clang-format in this PR is for demonstration only.

BTW. Every time I develop ThorVG I have to disable the editor's automatic code formatting behavior :(

@hermet
Copy link
Member

hermet commented Apr 18, 2024

@hermet Do you think this method is acceptable?

@RuiwenTang Firstly, I'm welcome to this. Here are two major points we need to make a consensus:

  • When to apply? This change will bring huge code modifications that may be difficult for maintainers of other systems to integrate with the newer versions of ThorVG. That's why I've considered the change point is at 1.0 release.
  • What coding convention standard do we prefer? There are a bunch of different coding standard by the well-known organizations/companies and ThorVG has no connection to Google so far.

I think it all depends on you. As the number of code and contributors increases, global formatting becomes increasingly expensive. Also, I think the current coding style might cause confusion when others are first contributing to ThorVG

We can discuss the details of coding style together, clang-format in this PR is for demonstration only.

BTW. Every time I develop ThorVG I have to disable the editor's automatic code formatting behavior :(

Firstly, many contributors face the same challenges when they contribute to an unfamiliar project, often making similar mistakes. This is not an issue unique to ThorVG. Rather, I would like to emphasize that while coding conventions for developers are a minor issue, integration issues with ThorVG will be more significant. Therefore, I am placing a bet on this for the 1.0 release. Otherwise, integrating it with large systems like Godot, LVGL, and Tizen could pose major challenges. Perhaps we could consider an auto-formatting script that aligns with the current ThorVG style. People could simply write the code first and run the script afterward. It's easy, and everyone will be happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Dev infrastructure refactoring Code refactoring / Exceptional handles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants