Skip to content

Add small documentation to parameter#15666

Merged
Siedlerchr merged 2 commits into
mainfrom
add-doc
May 3, 2026
Merged

Add small documentation to parameter#15666
Siedlerchr merged 2 commits into
mainfrom
add-doc

Conversation

@koppor

@koppor koppor commented May 3, 2026

Copy link
Copy Markdown
Member

I had a face-to-face discussion with a potential contributor. His background was more on Spring boot and the dependency injection going on there. The JabRef dependency injection was strange for him.

If no "magic" DI was in place (refs https://github.com/JabRef/jabref-issue-melting-pot/issues/590), the expectation were Factories. See https://en.wikipedia.org/wiki/Factory_method_pattern - this is kind of the same concern risen at the beginning of https://github.com/JabRef/jabref-issue-melting-pot/issues/590.

This PR adds a small documentation note on why the AI Service is needed at an "intuitively" strange place.

Steps to test

See CI passing

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • [/] I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

@koppor koppor added the dev: code-quality Issues related to code or architecture decisions label May 3, 2026
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add documentation comment for AIService parameter

📝 Documentation

Grey Divider

Walkthroughs

Description 
• Add documentation comment explaining AIService parameter usage
• Clarify dependency injection pattern for contributors unfamiliar with JabRef's approach
Diagram 
flowchart LR
  A["CitationFetcherType.getCitationFetcher()"] -- "documents AIService usage" --> B["PlainCitationParser dependency"]
Loading

Grey Divider

File Changes

1. jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java 📝 Documentation +1/-0

Document AIService parameter dependency requirement

• Added JavaDoc comment explaining the aiService parameter requirement
• Documents that AIService is needed for PlainCitationParser instantiation
• Clarifies non-obvious dependency injection pattern for new contributors

jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented May 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Malformed PlainCitationParser link ☑ 📘 Rule violation ⚙ Maintainability
Description 
The added Markdown documentation comment uses
[@org.jabref.logic.importer.plaincitation.PlainCitationParser], which is not the project’s
established doc-linking style and likely won’t render as a proper reference. This violates JabRef’s
code style conventions for documentation comments and reduces readability.
Code

jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java[35]

+    /// @param aiService required for [@org.jabref.logic.importer.plaincitation.PlainCitationParser]
Evidence 
PR Compliance ID 3 requires following JabRef code style conventions; the new doc comment introduces
a non-standard link format ([...]) instead of the consistently used {@link ...}-style references
in /// documentation comments.

AGENTS.md
jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java[35-35]

Agent prompt 
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new `///` documentation comment uses `[@org.jabref.logic.importer.plaincitation.PlainCitationParser]`, which is not the established/recognized doc-link format and may not render as a link.
## Issue Context
The codebase uses `///` documentation comments together with `{@link ...}` references elsewhere; keeping the same style improves readability and tooling support.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fetcher/citation/CitationFetcherType.java[35-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Grey Divider

Qodo Logo

@Siedlerchr

Siedlerchr commented May 3, 2026

Copy link
Copy Markdown
Member

Qodo is right, the markdown linking is not supported using [ ]

@Siedlerchr Siedlerchr enabled auto-merge May 3, 2026 17:28
@Siedlerchr Siedlerchr added this pull request to the merge queue May 3, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label May 3, 2026
Merged via the queue into main with commit 85500c9 May 3, 2026
56 of 57 checks passed
@Siedlerchr Siedlerchr deleted the add-doc branch May 3, 2026 18:06
Siedlerchr added a commit to FynnianB/jabref that referenced this pull request May 4, 2026
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into feature/jabsrv-secu…
b49185f 
…rity

* upstream/main: (204 commits)
  New Crowdin updates (JabRef#15669)
  Fix OpenRewrite (JabRef#15670)
  Udpate heylogs (and fix CHANGELOG.md) (JabRef#15671)
  Improve security and prevent shell injection for push2applications (JabRef#15628)
  Fix depdency analysis (JabRef#15668)
  Always use CI-local "gradle", instead of gradlew (JabRef#15667)
  Change OpenRewrite task to use rewriteDryRun (JabRef#15664)
  Add small documentation to parameter (JabRef#15666)
  Fix markbaseChanged for "imported entries" (JabRef#15610)
  Add forgotten --fresh
  chore(deps): update dependency com.github.ben-manes.caffeine:caffeine to v3.2.4 (JabRef#15662)
  chore(deps): update jackson monorepo to v3.1.3 (JabRef#15659)
  chore(deps): update dependency org.glassfish.hk2:hk2-utils to v4.0.1 (JabRef#15657)
  chore(deps): update dependency org.glassfish.hk2:hk2-locator to v4.0.1 (JabRef#15656)
  fix gemsfx missing icon resolving (JabRef#15655)
  chore(deps): update dependency org.glassfish.hk2:hk2-api to v4.0.1 (JabRef#15654)
  chore(deps): update dependency org.postgresql:postgresql to v42.7.11 (JabRef#15634)
  Chore(deps): Bump tools.jackson:jackson-bom in /versions (JabRef#15653)
  Chore(deps): Bump dev.langchain4j:langchain4j-bom in /versions (JabRef#15652)
  Chore(deps): Bump com.dlsc.gemsfx:gemsfx in /versions (JabRef#15651)
  ...
Siedlerchr added a commit that referenced this pull request May 5, 2026
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into add-oauth2
1292da7 
* upstream/main: (775 commits)
  Chore(deps): Bump com.konghq:unirest-modules-gson in /versions (#15682)
  Chore(deps): Bump org.glassfish.jaxb:jaxb-runtime in /versions (#15681)
  Update dependency com.konghq:unirest-modules-gson to v4.9.0 (#15679)
  Integrate with SearchRxiv  (#15373)
  Fix requirements (#15600)
  refactor: less objects during writing (#15677)
  Fix: suppress WARN for empty or blank column name in MainTableColumnModel#parse() (#15576)
  New Crowdin updates (#15676)
  Chore(deps): Bump com.github.ben-manes.caffeine:caffeine in /versions (#15673)
  Fix Nullwarnings - C (Mark bst package as nonnull by default) (#15663)
  Chore(deps): Bump com.github.javaparser:javaparser-symbol-solver-core (#15674)
  Chore(deps): Bump com.github.javaparser:javaparser-core in /versions (#15672)
  New Crowdin updates (#15669)
  Fix OpenRewrite (#15670)
  Udpate heylogs (and fix CHANGELOG.md) (#15671)
  Improve security and prevent shell injection for push2applications (#15628)
  Fix depdency analysis (#15668)
  Always use CI-local "gradle", instead of gradlew (#15667)
  Change OpenRewrite task to use rewriteDryRun (#15664)
  Add small documentation to parameter (#15666)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees

No one assigned

Labels

dev: code-quality Issues related to code or architecture decisions status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@koppor @Siedlerchr