Refactor PDF import #12310
Refactor PDF import #12310
Conversation
|
WIP Currently tests don't work |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
There was a problem hiding this comment.
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.
In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
There was a problem hiding this comment.
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.
In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
There was a problem hiding this comment.
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.
In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
There was a problem hiding this comment.
JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.
|
Couple of questions:
I'm asking these questions, as there are small inconsistencies in PDF importers |
There was a problem hiding this comment.
JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.
|
ID is for command line usage and is overwritten in all typical Importers, e..g MedlinePlainImporter , |
@InAnYan Please add this (slightliy updated) as JavaDoc! |
# Conflicts: # src/main/java/org/jabref/logic/importer/fileformat/pdf/PdfContentImporter.java
There was a problem hiding this comment.
JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.
There was a problem hiding this comment.
You modified Markdown (*.md) files and did not meet JabRef's rules for consistently formatted Markdown files.
To ensure consistent styling, we have markdown-lint in place.
Markdown lint's rules help to keep our Markdown files consistent within this repository and consistent with the Markdown files outside here.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "Markdown".
There was a problem hiding this comment.
You modified Markdown (*.md) files and did not meet JabRef's rules for consistently formatted Markdown files.
To ensure consistent styling, we have markdown-lint in place.
Markdown lint's rules help to keep our Markdown files consistent within this repository and consistent with the Markdown files outside here.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "Markdown".
|
Another tnice show case for refactoring miner:
|
# Conflicts: # build.gradle
There was a problem hiding this comment.
You modified Markdown (*.md) files and did not meet JabRef's rules for consistently formatted Markdown files.
To ensure consistent styling, we have markdown-lint in place.
Markdown lint's rules help to keep our Markdown files consistent within this repository and consistent with the Markdown files outside here.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "Markdown".
There was a problem hiding this comment.
JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.
You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.
|
DevCall decision: Fix conflicts and then this is good to go! 🎉 |
# Conflicts: # build.gradle # src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java # src/main/java/org/jabref/logic/importer/fileformat/pdf/PdfContentImporter.java
There was a problem hiding this comment.
Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".
Updates:
pdfpackage.PdfImporter(someimportDatabase,recognizedFormat, etc.).PDDocumentis sent toimportDatabasefunction of specific PDF importers, as they often use this type. Added in order to reduce duplicate work.I think people more drag-n-drop PDF files, rather click "Extract metadata from PDF", so I think this new merge dialog is useful (it's not new, just new place where it's opened 😄).
Mandatory checks
- [ ] Change inCHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)- [ ] Screenshots added in PR description (for UI changes)