Use git for backup #12252
Use git for backup #12252
Trag Code Review
Reviewed files details
Details
[2025-03-12T22:49:54.032Z] code review started
[2025-03-12T22:49:54.032Z] owner: JabRef
[2025-03-12T22:49:54.032Z] repo: jabref
[2025-03-12T22:49:54.032Z] repoUrl: https://github.com/JabRef/jabref
[2025-03-12T22:49:54.032Z] author: khola22
[2025-03-12T22:49:54.032Z] listing pull request files
[2025-03-12T22:49:54.694Z] file count: 18
[2025-03-12T22:49:54.757Z] pro user privilege applied
[2025-03-12T22:49:54.757Z] getting project rules
[2025-03-12T22:49:54.770Z] model: claude-3-5-sonnet-20240620
[2025-03-12T22:49:54.770Z] on rule review mode: true
[2025-03-12T22:49:54.770Z] glob ignore:
[2025-03-12T22:49:54.770Z] pull number: 12252
[2025-03-12T22:49:54.770Z] projectId: 885262de-6ea4-406f-ab33-0a18e077aaf9
[2025-03-12T22:50:07.060Z] file: CHANGELOG.md
[2025-03-12T22:50:07.060Z] not a code file, skipping
[2025-03-12T22:50:07.260Z] file: buildres/abbrv.jabref.org
[2025-03-12T22:50:07.261Z] not a code file, skipping
[2025-03-12T22:50:07.261Z] file: jabref
[2025-03-12T22:50:07.261Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:07.261Z] file: src/main/java/org/jabref/gui/LibraryTab.java
[2025-03-12T22:50:07.261Z] reading file blob
[2025-03-12T22:50:23.159Z] filteredRules: 1. If a method has JavaDoc and code of the method has changed, the JavaDoc has to be updated accordingly.
2. If code in org.jabref.model or org.jabref.logic has been changed, tests need to be adapted or updated accordingly
3. The code should follow the fail fast principle by immediately handling invalid states and returning early instead of nesting logic inside else branches.
Example:
Bad:
if (path.isEmpty()) {
return false;
} else {
// other code
}
Good:
if (path.isEmpty()) {
return false;
}
// other code
4. Assertion statements must not include the message parameter - the method name should already convey the expected behavior.
5. The pull request title should contain a short title of the issue fixed (or what the PR adresses) and not just "Fix issue xyz"
6. The "Mandatory checks" are Markdown TODOs. They should be formatted as that. Wrong: - [ x]
. Either - [ ]
or - [x]
.
7. New methods (and new classes) should follow the Single-responsibility principle (SRP).
8. There should be JavaDoc for complex methods.
9. "Magic" numbers or strings should be constants - or at least have a Java comment. Exception: JavaFX height and widths.
10. Avoid code duplication
11. Use modern Java best practices, such as Arguments.of() instead of new Object[] in JUnit tests, to improve readability and maintainability.
12. Exceptions should be used for exceptional states - not for normal control flow
13. Follow the principles of "Effective Java"
14. In JabRef, localized strings are done using Localization.lang("string")
. More information at https://devdocs.jabref.org/code-howtos/localization.html.
15. No use of Java SWING, only JavaFX is allowed as UI technology
16. @DisplayName for tests should only be used if absolutely necessary: The method name itself should be comprehensive enough.
17. Comments should add new information (e.g. reasoning of the code). It should not be plainly derived fro the code itself.
18. Instead of Files.createTempDirectory
@TempDir
JUnit5 annotation should be used.
19. Do not catch the general java java.lang.Exception. Catch specific exeptions only
20. Avoid exclamation marks at the end of a sentence. They are more for screaming. Use a dot to end the sentence.
21. All labels and texts should be sentence case (and not title case)
22. New public methods should not return null
. They should make use of java.util.Optional
. In case null
really needs to be used, the JSpecify annotations must be used.
23. Use "BibTeX" as spelling for bibtex
24. New strings should be consistent to other strings. They should also be grouped semantically together.
25. Existings strings should be reused instead of introducing slightly different strings
26. Comments on/above methods should be JavaDoc.not simple Java comments //. Three /// are OK (because this is Java23 and later)
27. The CHANGELOG.md entry should be for end users (and not programmers).
28. User dialogs should have proper button labels: NOT yes/no/cancel, but indicating the action which happens when pressing the button
29. GUI code should only be a gateway to code in org.jabref.logic. More complex code regarding non-GUI operations should go into org.jabref.logic. Think of layerd archicture.
30. null
should never be passed to a method (except it has the same name).
31. Do not add extra blank lines in CHANGELOG.md
32. Remove commented code. (To keep a history of changes git was made for.)
33. Do not use Objects.requireNonNull. Use JSpecify @NonNull
annotation if needs be.
[2025-03-12T22:50:36.261Z] filtering out non-relevant issues
[2025-03-12T22:50:36.659Z] broad list of issues found
[2025-03-12T22:50:36.659Z] score: 7
[2025-03-12T22:50:36.660Z] Reason: The method throws GitAPIException and IOException but there's no JavaDoc documenting these exceptions, which is required for complex methods involving external operations.
[2025-03-12T22:50:36.660Z] score: 8
[2025-03-12T22:50:36.660Z] Reason: Catches general Exception instead of specific exceptions (GitAPIException and IOException). This violates the principle of catching specific exceptions only.
[2025-03-12T22:50:36.660Z] score: 7
[2025-03-12T22:50:36.660Z] Reason: Method throws checked exceptions but lacks JavaDoc documentation for the exceptions. Complex methods should have comprehensive JavaDoc including @throws tags.
[2025-03-12T22:50:36.660Z] score: 7
[2025-03-12T22:50:36.960Z] Reason: Method signature changed to throw exceptions but JavaDoc wasn't updated to reflect these changes. When method changes, its documentation must be updated accordingly.
[2025-03-12T22:50:36.960Z] filtering out low importance issues
[2025-03-12T22:50:36.960Z] --------------------------------
[2025-03-12T22:50:36.960Z] writing comments to pr...
[2025-03-12T22:50:44.689Z] rule: Do not catch the general java java.lang.Exception. Catch specific exeptions only
[2025-03-12T22:50:44.689Z] comment: Catches general Exception instead of specific exceptions (GitAPIException and IOException). This vio...
[2025-03-12T22:50:46.077Z] rule: If a method has JavaDoc and code of the method has changed, the JavaDoc has to be updated accordingl...
[2025-03-12T22:50:46.078Z] comment: Method signature changed to throw exceptions but JavaDoc wasn't updated to reflect these changes. Wh...
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/backup/BackupChoiceDialog.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/backup/BackupChoiceDialogRecord.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/backup/BackupEntry.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/backup/BackupResolverDialog.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/dialogs/BackupUIManager.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/main/resources/csl-locales
[2025-03-12T22:50:46.078Z] not a code file, skipping
[2025-03-12T22:50:46.078Z] file: src/main/resources/csl-styles
[2025-03-12T22:50:46.078Z] not a code file, skipping
[2025-03-12T22:50:46.078Z] file: src/main/resources/l10n/JabRef_en.properties
[2025-03-12T22:50:46.078Z] not a code file, skipping
[2025-03-12T22:50:46.078Z] file: src/test/java/org/jabref/gui/autosaveandbackup/BackupManagerDiscardedTest.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/test/java/org/jabref/gui/autosaveandbackup/BackupManagerTest.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] file: src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java
[2025-03-12T22:50:46.078Z] file is outside of new changes on pr, skipping
[2025-03-12T22:50:46.078Z] files scanned: 1
[2025-03-12T22:50:46.078Z] lines scanned: 66
[2025-03-12T22:50:46.078Z] rules for project: 33
[2025-03-12T22:50:46.078Z] issues caught by your rules: 2