Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code style settings #535

Closed
wants to merge 3 commits into from
Closed

Code style settings #535

wants to merge 3 commits into from

Conversation

@theoriginalbit
Copy link

@theoriginalbit theoriginalbit commented Mar 30, 2018

Added code style settings for IntelliJ, that way when other developers contribute to this mod their code will be more likely to match the established code style.

The settings I've used are consistent with the bulk of the code I've seen throughout, though there were some inconsistencies that meant I got IntelliJ to reformat code to the style I setup. I recommend running some code cleanup over the existing code base.

Let me know if there are any other recommendations to add to the code style settings xml file.

@SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Mar 30, 2018

Just a couple of things which I feel are off, though I may be wrong:

  • Change continuation indent to 4 from 8. There's a lot of stuff like this and from what I remember the former is more common.
  • Is breaking javadoc @links across multiple lines valid?
  • I'm really not sure about ( m_payload != null ? m_payload.hashCode() : 0 ) (and parenthesis expression) having spaces. I know arguments do, but didn't think other expressions did.

It might also be worth adding an EditorConfig file to the root, and doing some whitespace cleanup on the Lua files. It'd also prevent missing trailing new-lines on new files looks at codeStyleSettings.xml.

My biggest worry/complaint about this, and what has stopped me in the past, is that it's going to cause merge conflicts with pretty much every PR out there. I think the better thing to do would be to set up the guidelines to ensure new code is written using them, and Dan can clean up the existing code at his leisure.

@theoriginalbit
Copy link
Author

@theoriginalbit theoriginalbit commented Mar 30, 2018

Change continuation indent to 4 from 8. There's a lot of stuff like this and from what I remember the former is more common.

Ohhhh. That's how I fix that one. I couldn't figure out which setting controlled that. Continuation indent, cool, I'll add it now.

Is breaking javadoc @links across multiple lines valid?

It should be, but I can double check by generating the Javadoc. Where's the one that's broken over multiple lines?

I'm really not sure about ( m_payload != null ? m_payload.hashCode() : 0 ) (and parenthesis expression) having spaces. I know arguments do, but didn't think other expressions did.

There's a lot of cases where other expressions had them. I'm not a fan of it, but having the spaces removed seemed to result in a lot more changes to the code.

It might also be worth adding an EditorConfig file to the root, and doing some whitespace cleanup on the Lua files. It'd also prevent missing trailing new-lines on new files looks at codeStyleSettings.xml.

Good idea. Didn't think about the Lua files.

My biggest worry/complaint about this, and what has stopped me in the past, is that it's going to cause merge conflicts with pretty much every PR out there.

From when I've done this in the past most of Git's conflict resolution should be ok with whitespace changes. I can definitely remove the commit where I performed the cleanup; I'll do that now. I mostly used that commit to prove that I wasn't drastically changing style and instead following what was established.

@SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Mar 30, 2018

It should be, but I can double check by generating the Javadoc. Where's the one that's broken over multiple lines?

A couple of files in the public API, I think IPeripheral had some? I'd assume it's OK, but GH's syntax highlighting complained so I just though I'd check.

There's a lot of cases where other expressions had them.

Ahhh, that's fine. I tried to have a look at a couple of random files to see what they did, but as you say, it's not very consistent :). It's rather embarrassing going through the reformatting diff and going "oooh, that's my code which is inconsistently formatted".

@theoriginalbit
Copy link
Author

@theoriginalbit theoriginalbit commented Mar 30, 2018

Ok found a multiline @link. Javadoc generates fine.

It's rather embarrassing going through the reformatting diff and going "oooh, that's my code which is inconsistently formatted".

It doesn't help that the code style largely used throughout isn't what's default to most Java projects nowadays and thus IntelliJ's defaults too.

@theoriginalbit theoriginalbit deleted the theoriginalbit:quality/code-style-settings branch Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants