Closed Bug 976353 Opened 6 years ago Closed 6 years ago

Install Eclipse formatting and code clean up defaults for AndroidEclipse backend generated projects

Categories

(Firefox Build System :: General, defect)

All
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla30

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(2 files)

Let's make new developers lives easier. We can iterate on these settings as needed.
BenWa has a template in bug 973770 for formatting. I think you should it.
Should share it. goto fail.
(In reply to Gregory Szorc [:gps] from comment #2)
> Should share it. goto fail.

Could do; BenWa's formatting is for the Eclipse CDT plugin, and this is for the JDT plugin.

Sharing such formatting between projects means writing things to the user's Eclipse workspace, and I have avoided doing so because it's quite tricky to arrange, and because it's a clear demarcation between what the backend owns and what the user owns.
The most important part here is 4 spaces, no tabs.  We can iterate on
other things over time.

The installation is "per project", when it could in theory be shared,
because it's quite difficult to actually arrange the sharing.  We'd need
to write into an Eclipse workspace (which we don't currently even know).
Attachment #8381031 - Flags: review?(rnewman)
This installs:

* Remove unused imports
* Add missing '@Override' annotations
* Add missing '@Override' annotations to implementations of interface methods
* Add missing '@Deprecated' annotations
* Remove unnecessary casts
* Remove unnecessary '$NON-NLS$' tags
* Organize imports
* Remove trailing white spaces on all lines

The installation is "per project", when it could in theory be shared,
because it's quite difficult to actually arrange the sharing.  We'd need
to write into an Eclipse workspace (which we don't currently even know).
Attachment #8381033 - Flags: review?(rnewman)
Comment on attachment 8381031 [details] [diff] [review]
Part 1: Make AndroidEclipse backend install formatting defaults. r=rnewman

Review of attachment 8381031 [details] [diff] [review]:
-----------------------------------------------------------------

rs

::: python/mozbuild/mozbuild/backend/templates/android_eclipse/.settings/org.eclipse.jdt.core.prefs
@@ +89,5 @@
> +org.eclipse.jdt.core.formatter.indent_statements_compare_to_block=true
> +org.eclipse.jdt.core.formatter.indent_statements_compare_to_body=true
> +org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_cases=true
> +org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_switch=false
> +org.eclipse.jdt.core.formatter.indentation.size=2

4?
Attachment #8381031 - Flags: review?(rnewman) → review+
Comment on attachment 8381033 [details] [diff] [review]
Part 2: Make AndroidEclipse backend install clean up defaults. r=rnewman

Review of attachment 8381033 [details] [diff] [review]:
-----------------------------------------------------------------

rs

::: python/mozbuild/mozbuild/backend/templates/android_eclipse/.settings/org.eclipse.jdt.ui.prefs
@@ +1,2 @@
>  #filter substitution
> +cleanup.add_default_serial_version_id=true

Recommend not.

@@ +9,5 @@
> +cleanup.add_missing_override_annotations_interface_methods=true
> +cleanup.add_serial_version_id=false
> +cleanup.always_use_blocks=true
> +cleanup.always_use_parentheses_in_expressions=false
> +cleanup.always_use_this_for_non_static_field_access=false

Oh, how I want this.

@@ +28,5 @@
> +cleanup.qualify_static_member_accesses_through_instances_with_declaring_class=true
> +cleanup.qualify_static_member_accesses_through_subtypes_with_declaring_class=true
> +cleanup.qualify_static_member_accesses_with_declaring_class=false
> +cleanup.qualify_static_method_accesses_with_declaring_class=false
> +cleanup.remove_private_constructors=true

?

@@ +32,5 @@
> +cleanup.remove_private_constructors=true
> +cleanup.remove_trailing_whitespaces=true
> +cleanup.remove_trailing_whitespaces_all=true
> +cleanup.remove_trailing_whitespaces_ignore_empty=false
> +cleanup.remove_unnecessary_casts=true

This seems worrisome.
Attachment #8381033 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/mozilla-central/rev/88a8926eb7cd
https://hg.mozilla.org/mozilla-central/rev/09240240ab18
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.