some refactoring #302
some refactoring #302
Conversation
|
Like #274, this serie improves the code consistency. |
|
I guess it's pretty clear that the 4 commits besides "avoid assignment inside condition" aren't worth a discussion. I added you guys as reviewers to comment on the advantages and disadvantages of keeping as is vs. merging the "avoid assignment inside condition" change. |
|
I would not merge the assignment commit. Besides that, it is fine with me. |
czurnieden
left a comment
czurnieden
left a comment
There was a problem hiding this comment.
Nice as always, @fperrad , thanks!
Regarding the "assignment" commit: that's a matter of style.
I have no style, you can even catch me wearing a bright green shirt in combination with dark blue denim, so don't drag me into it, I'll follow the majority here ;-)
But joking aside: LTM has no "style-sheet" beside the astyle-config. Should it have one (e.g.: a wiki page)? It would make it easier for new comitters like e.g.:@MasterDuke17 who asked me what I mean by "needs some formatting & style adaption" and I wasn't able to point them to a single point of information.
|
Well, the code style is with assignments as of now. There is no harm as it is. Why change it? I think I said it before - while I like consistency, I don't like pure style changes. They obfuscate the git history. The other commits in this PR are better, for example scope or error handling with goto - these improve the code. |
|
@fperrad can you please rebase and remove the 'avoid assignment inside condition' commit |
4 participants
No description provided.