Re: [hatari-devel] OSx problems |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
Hi Jerome,
Am Sun, 01 Jun 2014 18:28:03 +0200
schrieb Jerome Vernet <vernet.jerome@xxxxxxxxxx>:
> Hi Thomas,
>
> For paths.c, does this patch
> https://www.dropbox.com/s/mvi3b6e2ynulk3i/paths.c.diff
>
> is ok for you ?
That one worked (but there was still some unecessary code churn in
there which I removed, see below)
> I removed all C++ comments, also.
>
> Remark: there is elsewhere some C++ comment (in main.c, for example).
The problem is not the C++ comments themselves, but that you're changing
coding style along with functional changes in one patch. You should try
to avoid changing coding style in files that you've not written on your
own because:
1) Somebody wrote it that way and most likely liked it that way, so
might step on their toes when changing their coding style (yes, I know,
I also always think that my coding style is the best and I feel the
urge to change everything to my style, but when editing the code of
others, it's better to suppress that feeling ;-))
2) Someone (me in this case) has got to review your patch before
including it into the repository, so the more fuzz you have in your
patch, the harder it is to review it and to see what really has been
changed
3) Similar problem like 2) sometimes occurs later: When hunting bugs
later, you want to have a clean history. If you suspect that a bug has
been introduced in a certain revision, you do not want to have to
browse through many lines of coding style changes - no, you want to see
the functional changes as fast as possible.
4) The more lines you change, you increase the danger of conflicts with
other patches from other developers to the same file that might be work
in progress at the same time.
So please try to separate functional changes from coding style changes.
(and don't get me wrong, coding style cleanup can also be a good thing,
in case it has really been messed up in a file, but then it should be
fixed with a separate patch that does not contain functional changes).
Thanks,
Thomas