Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [ptp-dev] Code cleanup

Hi Greg,

On Thu, Jul 22, 2010 at 9:15 AM, Greg Watson <g.watson@xxxxxxxxxxxx> wrote:
  
I use a 132 character line length, as I find 80 characters results in too much line wrapping.

As far as I can tell, our formatting preferences are the same.

Assuming we agree on these, why not put the files on the wiki page to make it easy for people to use the same settings?
I'm fine with your settings and I agree they should be put on the wiki. I guess we should wait a fee days to see whether someone has any objections.

Regarding sorting members, I usually do this manually. I agree that it can make viewing changes more difficult (although some of the formatting changes can also do that). I don't have any objections if you want to change every source file, but it's going to be a big job.
Why would that be a big job? One just selects all projects and clicks clean-up. Or is it required to review the changes by the clean-up? I have never seen an error and thus stopped reviewing the changes.

If we don't want some of the clean-up steps (e.g. the source code formating as Beth just pointed out) to be done automatically, we could just do the clean-up for everything else.
 
Another approach might be to update the source for individual plugins as they are being worked on, prior to committing any fixes.
That doesn't work as well for patches. Because the person without committer access can't do this as well. Thus while this would also be an OK solution I would prefer doing all at one.

Roland

 
On Jul 22, 2010, at 1:16 AM, Roland Schulz wrote:

> Hi,
>
> according to http://wiki.eclipse.org/PTP/developer_guidelines the members should be sorted but often they are not. Thus if people clean-up the code before submitting a patch or committing code, than the patch/commit is significant larger than the change itself. This makes reviewing the change much more difficult than it should be.
>
> Thus I suggest to clean-up the whole code in one go. And in case it is decided that currently is not a good moment to do this change, I would suggest temporarily remove the recommendation to sort-members of existing function until this change is made at a later point.
>
> It might be sufficient to only do the member sorting in one go. Other clean-up changes have less of an effect on the readability of patches. But
> I would suggest to also do the other clean-up changes in the same step.
>
> In this context, I would like to clarify what the recommended setting for formatter and clean-up are. Do we use 80 or 132 line-length?
> Greg asked me to use 132 in 315713 but I'm not sure whether it was meant only in that context or whether this is in general the recommendation.
>
> For the clean-up I use the default of:
> Change non static accesses to static members using declaring type
> Change indirect accesses to static members to direct accesses (accesses through subtypes)
> Remove unused imports
> Add missing '@Override' annotations
> Add missing '@Deprecated' annotations
> Remove unnecessary casts
> Remove unnecessary '$NON-NLS$' tags
> WITHOUT:
> Sort members excluding fields, enum constants, and initializers
> PLUS:
> Organize imports
> Format source code
> Remove trailing white spaces on all lines
> Correct indentation
>
> It this the recommended setting?
>
> I have attached the xml with the formatter and clean-up settings as described here in the text.
>
>
> Roland
>
>
> --
> ORNL/UT Center for Molecular Biophysics cmb.ornl.gov
> 865-241-1537, ORNL PO BOX 2008 MS6309
> <cleanup.xml><formatter.xml>_______________________________________________
> ptp-dev mailing list
> ptp-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/ptp-dev

_______________________________________________
ptp-dev mailing list
ptp-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/ptp-dev



--
ORNL/UT Center for Molecular Biophysics cmb.ornl.gov
865-241-1537, ORNL PO BOX 2008 MS6309

Back to the top