search

spotify / heroic

The Heroic Time Series Database
https://spotify.github.io/heroic/
Apache License 2.0
848 stars 109 forks source link

Codestyle: update from 6.x to 8.x #720

Closed sming closed 3 years ago

sming commented 3 years ago

TLD/checkstyle.xml does not contain rules that would flag the below code. We should add them.


import javax.ws.rs.core.MediaType;
    import org.apache.commons.lang3.tuple.Triple; // wrong indent

// too many lines between statements

@Path("query")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
public class QueryResource {
        private final JavaxRestFramework httpAsync;   // wrong indent
    private final QueryManager query;
sming commented 3 years ago

Hi @spotify/prism (sorry if this is irrelevant to you, I couldn't be arsed typing in ~10 names), please add your pet peeves with our current checkstyle config e.g. "I hate more than one parameter a line!!!" or "we must stop unsorted imports. It is an abomination."

sming commented 3 years ago

So I've managed to implement most of the above, apart from inconsistent indentation in import statements - see my SO question.

I'll commit my changes for now and give it until Monday 23rd for an answer on SO. If no answer, I'll call this done.

sming commented 3 years ago

I am putting this back in To-Do because it wasn't as straightforward as it seemed and the stability work must be wrapped up within 2 sprints (IIRC).

sming commented 3 years ago

OK. So. Adding the two checkstyle directives/modules (Indentation and EmptyLineSeparator) spawned literally hundreds of errors in the heroic-component module alone :(

Hence I will create a second issue to fix all the errors. That will take a fair bit of manual work!