Skip to content

chore: format java code#23

Merged
invaderb merged 11 commits intoUSTakAssociation:devfrom
Christian-Schefe:format-all-java
Sep 11, 2025
Merged

chore: format java code#23
invaderb merged 11 commits intoUSTakAssociation:devfrom
Christian-Schefe:format-all-java

Conversation

@Christian-Schefe
Copy link
Copy Markdown
Contributor

Code should be formatted for git diffs of my future commits to be readable. No idea why the code isn't formatted in the first place.
I haven't modified any code, I just ran the intellij idea formatter on all java files in server/src/main.

@nitzel
Copy link
Copy Markdown
Collaborator

nitzel commented Sep 9, 2025

I think the main changes are converting tabs to spaces here.

@Christian-Schefe
Copy link
Copy Markdown
Contributor Author

I think the main changes are converting tabs to spaces here.

That is most definitely a big part of the diffs, but there are also countless places with no proper spacing e.g. between operands: "a+b" instead of "a + b", and also the doc comments were expanded from one line into three. Also different line breaking at some places

Copy link
Copy Markdown
Collaborator

@nitzel nitzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. A few places the formatting has made the readability slightly worse (e.g. switch), but I approve moving from tabs to spaces as well as some more uniformity for the usage of spaces in conditions and expressions.

However

if we ever plan on moving forward with #7 then these changes will probably make merging changes a lot more annoying.

int rematchId;
public static enum COLOR {WHITE, BLACK, ANY}

;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what kind of formatting is this where the semicolon gets pushed 2 lines down?

Ah, public static enum NAME { Fields.. } doesn't need a semicolon at all. I guess removing the semicolon here would make sense, though it doesn't change anything ^^

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that, but I wanted to clean up all the places where Intellij complains anyways in another pull request (after formatting is done, so one can actually review the diff). It currently shows me 245 warnings for the java code alone. This actually also includes most of the switches that can be turned into switch expressions instead. Also there are places where C-style arrays are used (String value[]) instead of (String[] value), I didn't even know that's possible.

@Christian-Schefe
Copy link
Copy Markdown
Contributor Author

if we ever plan on moving forward with #7 then these changes will probably make merging changes a lot more annoying.

How so? From what I can see, that PR is exclusively TypeScript code, which I haven't touched at all. I don't think there is even a single merge conflict between the two PRs

@Christian-Schefe
Copy link
Copy Markdown
Contributor Author

I have now reverted to tabs as per bcreature's request. I have added a .editorconfig for consistent settings across editors (some settings are intellji-only, don't know if there are vscode equivalents, also vscode needs a plugin for .editorconfig iirc).
I optimized and reordered the imports and changed some minor code style issues.
The most notable ones are the c-style array syntax, unused exceptions in throw declarations, redundant final modifiers and that one comparison 0 >= sourceCount turned into sourceCount == 0 (as sourceCount is non-negative).

@invaderb invaderb changed the base branch from main to dev September 11, 2025 14:44
Copy link
Copy Markdown
Member

@invaderb invaderb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good approved

@invaderb invaderb merged commit 99b1d65 into USTakAssociation:dev Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants