Skip to content

Conversation

@joethei
Copy link
Contributor

@joethei joethei commented Nov 30, 2020

As discussed in #6627 (comment)

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Looks good to me

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 30, 2020
Copy link
Contributor

@DominikVoigt DominikVoigt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,
Thanks for your contribution!
I only have one question otherwise LGTM!

Comment on lines +173 to +176
@Override
public void doPostCleanup(BibEntry entry) {
// do nothing
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you overwrite this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as both SearchBased and IdBased Fetchers have a doPostCleanup default impl, the compiler is not sure which one to call

Copy link
Contributor

@DominikVoigt DominikVoigt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification! :)

@DominikVoigt DominikVoigt merged commit 706865f into JabRef:master Dec 1, 2020
Siedlerchr added a commit that referenced this pull request Dec 5, 2020
* upstream/master: (36 commits)
  Fix remembering password for sql db (#7154)
  Update to libre office 7.0.3 (#7150)
  Add IdBasedSearchFetcher to jstor (#7145)
  Squashed 'src/main/resources/csl-styles/' changes from 55200d0..a20406d
  Bump antlr4-runtime from 4.8-1 to 4.9 (#7136)
  Bump antlr4 from 4.8-1 to 4.9 (#7138)
  Bump mariadb-java-client from 2.7.0 to 2.7.1 (#7134)
  Bump classgraph from 4.8.90 to 4.8.92 (#7139)
  Bump mockito-core from 3.6.0 to 3.6.28 (#7135)
  Bump gittools/actions from v0.9.6 to v0.9.7 (#7144)
  Bump checkstyle from 8.37 to 8.38 (#7142)
  Add missing author
  Fix document viewer not showing first page (#7132)
  Add githandler mock to crawler test to fix NPE (#7133)
  Searchbar glyph icon colors in Dark Theme [FIXED] (#7131)
  Fix binding issue for the regex and case sensitive search buttons (#7125)
  Enable automated cross library search using a cross library query lan… (#7124)
  Add tracking
  Update Java Version
  Welcome Dominik ✌
  ...
Siedlerchr added a commit that referenced this pull request Dec 6, 2020
* upstream/master:
  Improve library loading UX (#7119)
  remove jython (#7157)
  Add missing authors
  Remove obsolete hint
  Fixed issue in PreviewView for number textfield (#7158)
  Fix for issue 6959 (#7151)
  Revert "remove jython (#7155)" (#7156)
  remove jython (#7155)
  Fix remembering password for sql db (#7154)
  Update to libre office 7.0.3 (#7150)
  Add IdBasedSearchFetcher to jstor (#7145)
  Squashed 'src/main/resources/csl-styles/' changes from 55200d0..a20406d
  Bump antlr4-runtime from 4.8-1 to 4.9 (#7136)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants