Skip to content

Issue676 author last#13625

Merged
Siedlerchr merged 6 commits into
JabRef:mainfrom
espertusnu:issue676-authorLast
Oct 20, 2025
Merged

Issue676 author last#13625
Siedlerchr merged 6 commits into
JabRef:mainfrom
espertusnu:issue676-authorLast

Conversation

@espertusnu

@espertusnu espertusnu commented Aug 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Addresses JabRef#676

  • Moves tests of lastAuthor from CitationKeyGeneratorTest to BracketedPatternTest
  • Fixes a link from javadoc to documentation
  • Reorders methods in BracketedPatternTest so argument providers consistently appear before tests using them

I made these changes as preparation for a class session in mid-September where I will demonstrate how to make a change and PR. It would be easiest for me if this is reviewed but not merged; however, if it is easier for you to merge once approved, I can create a pre-merge branch for the class demonstration.

The first commit message has a typo; it should be "Add test of [authLast]".

Steps to test

Rerun the tests in logic/citationkeypattern.

Mandatory checks

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

@koppor koppor mentioned this pull request Aug 1, 2025
Closed
koppor
koppor reviewed Aug 1, 2025
View reviewed changes
Comment thread jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java
Comment thread jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java Outdated
Comment thread jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java
@espertusnu espertusnu marked this pull request as ready for review August 2, 2025 02:39
@koppor

koppor commented Aug 2, 2025

Copy link
Copy Markdown
Member

Tests are failing due to missing dependencies:

Could not determine the dependencies of task ':jabls:checkstyleMain'.
> Could not resolve all dependencies for configuration ':jabls:compileClasspath'.
   > Could not resolve com.google.code.gson:gson:[2.9.1,3.0).
     Required by:
         project :jabls > com.github.eclipse:lsp4j:0.24.0 > com.github.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:0.24.0
         project :jabls > com.github.eclipse:lsp4j:0.24.0 > com.github.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc.debug:0.24.0
      > Failed to list versions for com.google.code.gson:gson.
         > Unable to load Maven meta-data from https://oss.sonatype.org/content/groups/public/com/google/code/gson/gson/maven-metadata.xml.
            > Could not GET 'https://oss.sonatype.org/content/groups/public/com/google/code/gson/gson/maven-metadata.xml'.
               > Read timed out

@koppor

koppor commented Aug 2, 2025

Copy link
Copy Markdown
Member

Note that not all tasks from JabRef#676 are adressed, but I think, this is on purpose :)

@Siedlerchr

Siedlerchr commented Aug 2, 2025

Copy link
Copy Markdown
Member

oss sonatype is down

@espertusnu

espertusnu commented Aug 2, 2025

Copy link
Copy Markdown
Contributor Author

Note that not all tasks from JabRef#676 are adressed, but I think, this is on purpose :)

Correct. You can close this PR when it's a good model for my students.

@jabref-machine

jabref-machine commented Sep 8, 2025

Copy link
Copy Markdown
Collaborator

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).

@trag-bot

trag-bot Bot commented Sep 8, 2025

Copy link
Copy Markdown

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

@koppor koppor marked this pull request as draft September 8, 2025 20:17
@espertusnu espertusnu mentioned this pull request Sep 16, 2025
Closed
@espertusnu espertusnu marked this pull request as ready for review October 7, 2025 21:00
@Siedlerchr Siedlerchr added this pull request to the merge queue Oct 20, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025
@Siedlerchr Siedlerchr added this pull request to the merge queue Oct 20, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025
@Siedlerchr Siedlerchr merged commit 72d9c1d into JabRef:main Oct 20, 2025
34 checks passed
Siedlerchr added a commit that referenced this pull request Oct 20, 2025
@Siedlerchr
Revert "Issue676 author last (#13625)"
835f346 
This reverts commit 72d9c1d.
@Siedlerchr Siedlerchr mentioned this pull request Oct 20, 2025
Merged
Siedlerchr added a commit that referenced this pull request Oct 20, 2025
@Siedlerchr
Revert "Issue676 author last (#13625)" (#14123)
5a9f512 
This reverts commit 72d9c1d.
@Siedlerchr

Siedlerchr commented Oct 20, 2025

Copy link
Copy Markdown
Member

@espertusnu Can you please re-open the PR, I accidentally merged it and did not see that some tests were failing.

Siedlerchr added a commit to turhantolgaunal/jabref that referenced this pull request Oct 20, 2025
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into newstudyyaml
cd4e27f 
* upstream/main:
  Revert "Issue676 author last (JabRef#13625)" (JabRef#14123)
  Issue676 author last (JabRef#13625)
@espertusnu

espertusnu commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

I don't have the open of re-opening this PR so I'll create a new one with the changes, after making sure all tests pass.

@espertusnu espertusnu mentioned this pull request Oct 21, 2025
Merged
2 tasks
merlinymy pushed a commit to merlinymy/jabref that referenced this pull request Nov 19, 2025
@espertusnu @Siedlerchr @merlinymy
Issue676 author last (JabRef#13625)
a4d8fb8 
* Add test of [authList]

* Consistently order argument providers and tests

* Move remaining lastAuthor tests

* Incorporate reviewer feedback

---------

Co-authored-by: espertusnu <espertusnu@users.noreply.github.com>
Co-authored-by: Christoph <siedlerkiller@gmail.com>
merlinymy pushed a commit to merlinymy/jabref that referenced this pull request Nov 19, 2025
@Siedlerchr @merlinymy
Revert "Issue676 author last (JabRef#13625)" (JabRef#14123)
c573ec1 
This reverts commit 72d9c1d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@koppor koppor koppor left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees

No one assigned

Labels

📌 Pinned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@espertusnu @koppor @Siedlerchr @jabref-machine