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

Code style inconsistencies #98

Closed
floriankramer opened this issue Aug 15, 2018 · 2 comments
Closed

Code style inconsistencies #98

floriankramer opened this issue Aug 15, 2018 · 2 comments
Projects

Comments

@floriankramer
Copy link
Member

floriankramer commented Aug 15, 2018

Every couple of pull requests when running clang-format on the entire repository I tend to get a lot of changed files.
I'd suggest adding an automated code check at some point in the pull request verification process. Personally I think the best place would be git's pre-commit hook, but as far as I know that cannot be synchronized automatically using git. I've attached a working pre-commit hook though, in case anybody is interested.
pre-commit
A less elegant solution would be checking the code style during the travis ci run. While this does not provide immediate feedback that the code style is off it would be fully automatic and does not require any setup by the user.
To my knowledge github does not support server sided hooks for non enterprise projects.
@niklas88 do you have any good ideas on how to improve code style conformity?

@niklas88
Copy link
Member

Hmm very good point. I guess I also forgot running clang-format a couple of time even though it's bound to :Fmt in my editor.

So ideally, I think we should all install this hook (nice work btw. I already installed it) AND have a GitHub Check either in Travis (relatively easy though Travis' ancient versions will surely create some headaches) or as a separate service.

Would you be willing to create a PR that adds the hook script to the repository (either in misc or in a separate directory) and also add a section in the README.md about installing it. Make it sound mandatory. @joka921 install that hook. The README is getting really long though so we should also think about whether we might want to split it up.

Then I'll have a try at making Travis run this check as well.

@niklas88
Copy link
Member

@floriankramer in your pre-commit hook the grep needs to be (cpp|h) instead of [cpp|h]

QLever automation moved this from In progress to Done Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
QLever
  
Done
Development

No branches or pull requests

2 participants