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

Add Checkstyle settings for newline after catch brace #12189

Merged
merged 9 commits into from
Nov 16, 2024

Conversation

subhramit
Copy link
Collaborator

@subhramit subhramit commented Nov 12, 2024

Since this has been troubling for a while now...
IntelliJ:
image

This PR should test for github actions and reviewdog before merge.

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@subhramit subhramit changed the title [Test] Add settings for newline after catch brace [Test] Add Checkstyle settings for newline after catch brace Nov 12, 2024
@subhramit
Copy link
Collaborator Author

"Test failed successfully"
But why does reviewdog not bark...

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

@subhramit
Copy link
Collaborator Author

Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues.

Ah, perfect.

@subhramit subhramit changed the title [Test] Add Checkstyle settings for newline after catch brace Add Checkstyle settings for newline after catch brace Nov 12, 2024
@subhramit subhramit added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 12, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

@Siedlerchr
Copy link
Member

Seems to work, but would be even better if we could get this fixed in IJ

Siedlerchr
Siedlerchr previously approved these changes Nov 12, 2024
@subhramit
Copy link
Collaborator Author

subhramit commented Nov 12, 2024

Seems to work, but would be even better if we could get this fixed in IJ

Do you mean the warning:
image

Or the auto-formatting that sometimes causes this?

@koppor
Copy link
Member

koppor commented Nov 12, 2024

Or the auto-formatting that sometimes causes this?

Yes, the auto formatting. Takes some "minutes" to experiment / google / check intellij issues / report intellij issue / ...

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

@koppor
Copy link
Member

koppor commented Nov 12, 2024

For completeness, I would like to see a link to an stackoverflow question having this as question - and posting this as answer maybe?

@subhramit
Copy link
Collaborator Author

For completeness, I would like to see a link to an stackoverflow question having this as question - and posting this as answer maybe?

Clarifying -
This as a question - by "this" do you mean the issue framed as a question? Like "how to configure intellij not to add a newline after..."

This as answer - you mean the checkstyle properties in this PR as the answer?

calixtus
calixtus previously approved these changes Nov 13, 2024
@subhramit
Copy link
Collaborator Author

Fixed the violations in the rest of the codebase. Tests will be green now. Can do a final review and merge.

@koppor
Copy link
Member

koppor commented Nov 16, 2024

For completeness, I would like to see a link to an stackoverflow question having this as question - and posting this as answer maybe?

Clarifying - This as a question - by "this" do you mean the issue framed as a question? Like "how to configure intellij not to add a newline after..."

Question: Title: "How can checkstyle be configured to throw an error if a line break is after the opening brace"

Example:

I have

try {
    // ...
} catch (
  Exception e) {
    // ...
}

Then you can also do "answer your own question". - Or wait for 48h if someone else answers

@koppor koppor added this pull request to the merge queue Nov 16, 2024
Merged via the queue into JabRef:main with commit fd6d06e Nov 16, 2024
23 checks passed
@koppor koppor deleted the checkstyle branch November 16, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants