GSIP 228
Switch from the current Google Java format AOSP profile, to the Palantir one.
Andrea Aime
This proposal is for GeoServer 2.27, 2.26 and 2.25
- Under Discussion
- In Progress
- Completed
- Rejected
- Deferred
The current auto-formatting has various benefits, but also some downsides. In particular, GeoTools, GeoWebCache and GeoServer code tend to have long variable names, long method names, and plenty of nested code (e.g., anonymous inner classes).
The Google Java format AOSP style definitions use 4 indents with just 80 columns, and were not designed to handle the code with streams/lambda usage.
The Palantir Java format is a fork of Google Java format designed to be a "modern, lambda-friendly, 120 character Java formatter."
120 columns are a better fit for long method names, variable names, and 4 spaces indent. The different treatment of method chains, with specific optimizations for streams and lambdas, helps keeping the code compact, yet readable.
Draft pull requests have been prepared, that show how the code would look when formatted with Palantir:
While the PRs visible online, the sheer number of modified files makes the Github pages very slow. You might want to check them out locally for more inspection.
Here are a few examples from the GeoServer re-formatting, for your convenience.
Random log statement:
- LOGGER.log(
- Level.SEVERE,
- "Could not get the list of output formats supported by gdal_translate",
- e);
+ LOGGER.log(Level.SEVERE, "Could not get the list of output formats supported by gdal_translate", e);Optional:
// write the metrics out to the cache
- Optional.ofNullable(RequestMetricsCallback.metrics.get())
- .ifPresent(
- map -> {
- RequestMetricsController.METRICS.put(reqId, map);
- RequestMetricsCallback.metrics.remove();
- });
+ Optional.ofNullable(RequestMetricsCallback.metrics.get()).ifPresent(map -> {
+ RequestMetricsController.METRICS.put(reqId, map);
+ RequestMetricsCallback.metrics.remove();
+ });
Streams:
- List<TemplateBuilder> filtered =
- children.stream()
- .filter(b -> b instanceof DynamicValueBuilder || b instanceof SourceBuilder)
- .collect(Collectors.toList());
+ List<TemplateBuilder> filtered = children.stream()
+ .filter(b -> b instanceof DynamicValueBuilder || b instanceof SourceBuilder)
+ .collect(Collectors.toList());Wicket anonymous inner classes:
- PanelCachingTab previewTab =
- new PanelCachingTab(
- new AbstractTab(new Model<>("Preview")) {
- @Override
- public Panel getPanel(String id) {
- previewPanel =
- new TemplatePreviewPanel(
- id, TemplateConfigurationPage.this);
- return previewPanel;
- }
- });
- PanelCachingTab dataTab =
- new PanelCachingTab(
- new AbstractTab(new Model<>("Data")) {
- @Override
- public Panel getPanel(String id) {
- return dataPanel =
- new TemplateInfoDataPanel(
- id, TemplateConfigurationPage.this) {
- @Override
- protected TemplatePreviewPanel getPreviewPanel() {
- return previewPanel;
- }
- };
- }
- });
+ PanelCachingTab previewTab = new PanelCachingTab(new AbstractTab(new Model<>("Preview")) {
+ @Override
+ public Panel getPanel(String id) {
+ previewPanel = new TemplatePreviewPanel(id, TemplateConfigurationPage.this);
+ return previewPanel;
+ }
+ });
+ PanelCachingTab dataTab = new PanelCachingTab(new AbstractTab(new Model<>("Data")) {
+ @Override
+ public Panel getPanel(String id) {
+ return dataPanel = new TemplateInfoDataPanel(id, TemplateConfigurationPage.this) {
+ @Override
+ protected TemplatePreviewPanel getPreviewPanel() {
+ return previewPanel;
+ }
+ };
+ }
+ });Switch to Palantir as the default GeoTools/GeoWebCache/GeoServer formatter. The switch will be applied simultaneously on development, stable, and maintenance branches to ease cherry-picking in backports.
Backporting to older, no longer supported branches, may become harder.
Project Steering Committee:
- Alessio Fabiani: +1
- Andrea Aime: +1
- Ian Turton: +1
- Jody Garnett: +1
- Jukka Rahkonen:
- Kevin Smith:
- Simone Giannecchini: +0
- Torben Barsballe: +1
- Nuno Oliveira: +1
- Peter Smythe: +1