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

convert Inner classes to record classes #11838

Merged

Conversation

leaf-soba
Copy link
Contributor

@leaf-soba leaf-soba commented Sep 26, 2024

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • 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.

- Inner classes like ComparableMark can be converted to record classes.
- most of them can't change it since there are a method in it or changed the record value, which against the purpose of record.
Comment on lines 97 to 98
* group : A list of consecutive citation groups only separated by spaces.
* groupCursor : A cursor covering the XTextRange of each entry in group (and the spaces between them)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this @param?

@koppor koppor added the type: code-quality Issues related to code or architecture decisions label Sep 26, 2024
fix missing at param
@leaf-soba leaf-soba requested a review from koppor September 26, 2024 14:57
this.groupCursor = groupCursor;
}
/**
* @param group : A list of consecutive citation groups only separated by spaces.
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the correct syntax

https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#format

Discussion at https://stackoverflow.com/a/16138292/873282

If you want, you can search whether checkstyle supports JavaDoc checks or if there are other ways to check for wrong syntax.

Note that IntelliJ should align the text - which is not the case in your thing. If you mark the two lines and press Ctrl+Alt+L, what happens?

fix javdoc format
@leaf-soba leaf-soba requested a review from koppor September 27, 2024 01:01
@koppor
Copy link
Member

koppor commented Sep 27, 2024

I don't see a merge queue option here. All tests are green. Thus, I am directly merging.

@koppor koppor merged commit 773f97a into JabRef:oo-maintenance Sep 27, 2024
23 checks passed
@koppor
Copy link
Member

koppor commented Sep 27, 2024

@subhramit The update did go into your oo-maintenance branch. I hope, that there won't be any conflicts when merging this branch into main. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openoffice/libreoffice type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants