-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add .clang-format #404
base: master
Are you sure you want to change the base?
Add .clang-format #404
Conversation
This formatting looks good. It's easy to read. This should be a trivial change as it should not break any features in the software. |
@@ -124,7 +123,7 @@ static inline size_t GetSystemPageSize() | |||
page_size = sSysInfo.dwPageSize; | |||
#elif defined(PAGESIZE) // defined in limits.h | |||
page_size = PAGESIZE; | |||
#else // assume some POSIX OS | |||
#else // assume some POSIX OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eww
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's that bad it's aligning with another comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's where we disagree, excessive white space is never good in my opinion. Personally I'm not for aligning trailing comments, there's literally no reason for it. Trailing comments are almost never in any way related to each other.
I like it apart from my inline comments (unfortunately I don't know enough about clangformat to know if it's possible to implement any of those fixes and doing them manually just means that it will revert them the next time it's ran). |
If we chose to use clang on the entire repo; we'd have a lot to go through and sort. It'd be worth it for future. A lot of the code within Paycoin is currently messy and could do with looking smarter. |
@jwrb I agree with the exception of my comments. |
Format the QT files using clang-format This file is based upon @MitchellCash's PR [PaycoinFoundation#404](PaycoinFoundation#404) It changes the formatting in a minor way. This is applied to the QT subdirectory as a whole. If any issues are found please report them.
I didn't want to break everything by doing all files.
299e4bf
to
9ffb345
Compare
Hmm I knew this was possibly going to be an issue with my pull but my clang-fu isn't up to scratch. I'll have to investigate those comments further but I'm not sure if it's possible or not and if we do find a fix the there is also the possibility that the fix might actually cause imperfections in other places. Could be a lesser of two evils type commit. Let me know your thoughts before I dive in too deep... |
@MitchellCash that's why I generally don't rely entirely on format tools. They're great for fixing format sometimes but once you've got the bulk of it fixed there's almost always something you want to tweak manually afterwards lol |
Agreed. I don't mind either way. Do we want this or not? |
I'd still prefer right aligned pointers. @IngCr3at1on? |
@jwrb semi-indifferent but that's how I would normally write them. |
@IngCr3at1on Agreed. I always use right-side pointers when I'm working. It looks nicer in my opinion too. |
This includes (requires) #403.
The idea to introduce .clang-format was based off a discussion in #394.
I have applied clang-format on a handful of files to show the effects (I didn't want to break everything by doing all files).
Command line: