jabref icon indicating copy to clipboard operation
jabref copied to clipboard

Refactor: CitaviXMLImporter to use StAX parser exclusively

Open ganesh-vk opened this issue 6 months ago • 7 comments

Closes #9539

Refactor: CitaviXMLImporter now uses StAX exclusively, deprecating reliance on JAXB and also improving performance. All tests from CitaviXmlImporterFilesTest pass successfully.

Steps to test

Run CitaviXmlImporterFilesTest or import a citavi file from jablib/src/test/resources/org/jabref/logic/importer/fileformat

image image

  • [x] I own the copyright of the code submitted and I license 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)
  • [x] Manually tested changed features in running JabRef (always required)
  • [x] Screenshots added in PR description (if change is visible to the user)
  • [x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [x] 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.

ganesh-vk avatar Jun 11 '25 23:06 ganesh-vk

You have removed the "Mandatory Checks" section from your pull request description. Please adhere to our pull request template.

jabref-machine avatar Jun 11 '25 23:06 jabref-machine

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

jabref-machine avatar Jun 11 '25 23:06 jabref-machine

Also please restore the "How to test" section in the PR description

subhramit avatar Jun 12 '25 16:06 subhramit

I'm fine with the change and all tests pass. the citativ tasks Needs to be removed from the gradle scripts now as well

Siedlerchr avatar Jun 13 '25 07:06 Siedlerchr

Your code currently does not meet JabRef's code guidelines. We use OpenRewrite to ensure "modern" Java coding practices. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / OpenRewrite (pull_request)" and click on it.

The issues found can be automatically fixed. Please execute the gradle task rewriteRun from the rewrite group of the Gradle Tool window in IntelliJ, then check the results, commit, and push.

jabref-machine avatar Jun 13 '25 16:06 jabref-machine

I am not sure if this code is more readable and maintainable than the old one.

imho it is. it reduces complexity a lot by easing the build tasks and reducing magic mappings.

please introduce string constants to replace the magic strings, then we can merge.

calixtus avatar Jun 15 '25 12:06 calixtus

@trag-bot didn't find any issues in the code! ✅✨

trag-bot[bot] avatar Jun 18 '25 19:06 trag-bot[bot]