-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Modularize JabSrv #13908
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
Modularize JabSrv #13908
Conversation
| @Override | ||
| public Void call() throws InterruptedException { | ||
| // The server serves the last opened files (see org.jabref.http.server.LibraryResource.getLibraryPath) | ||
| // The server serves the last opened files (see org.jabref.http.server.resources.LibraryResource.getLibraryPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment does not add new information or reasoning to the code. It merely restates what the code is doing, which is discouraged.
|
@trag-bot didn't find any issues in the code! ✅✨ |
| try { | ||
| libraryAsString = Files.readString(library); | ||
| } catch (IOException e) { | ||
| LOGGER.error("Could not read library {}", library, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try block covers too many statements. It should only cover the statement that might throw the exception, improving readability and maintainability.
| * @return a stream to the Chocolate.bib file in the classpath (is null only if the file was moved or there are issues with the classpath) | ||
| */ | ||
| private @Nullable InputStream getChocolateBibAsStream() { | ||
| return BibDatabase.class.getResourceAsStream("/Chocolate.bib"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning null from a method is discouraged. Consider using Optional to avoid potential NullPointerExceptions.
| */ | ||
| @GET | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| public String getJabMapJson(@PathParam("id") String id) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method returns a JSON string directly, which could be null if the file reading fails. It should return an Optional to avoid returning null.
| } | ||
|
|
||
| private java.nio.file.Path getJabMapDemoPath() { | ||
| java.nio.file.Path result = java.nio.file.Path.of(System.getProperty("java.io.tmpdir")).resolve("demo.jmp"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of System.getProperty("java.io.tmpdir") is correct, but the path should be managed using modern Java best practices like Path.of() instead of Paths.get().
Pull Request is not mergeable
* upstream/main: Add new check for format (#13909) Consistent casing in fieldnames (#13867) Revert "Pressing TAB in last field in entry editor moves focus to the next ta…" (#13912) Fix YAML Fix on-pr-opened-updated.yml syntax Pressing TAB in last field in entry editor moves focus to the next tab's first field (#13870) Modularize JabSrv (#13908) New translations jabref_en.properties (Italian) (#13907) Remove wrong `assert` statement (#13906) Add .git-blame-ignore-revs (#13884) Do not show transprot info messages (#13904) Pubmed api key support (#13899) Fix warnings for native access Fix automerge workflow (#13903) Add comment on issue on binary (#13902) Have checkstyle and VCS configuration distributed (#13900) Add unknown field to lsp consistency check (#13880) Put config for general tab if missing (#13901) Fix autosave manager exception on shutdown (#13882)
Makes JabSrv more modular
Steps to test
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)