Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add log for ignored excepton #9302

Merged
merged 13 commits into from
Dec 19, 2022
Merged

Add log for ignored excepton #9302

merged 13 commits into from
Dec 19, 2022

Conversation

VernonKwa
Copy link
Contributor

@VernonKwa VernonKwa commented Oct 24, 2022 •
edited by ThiloteE
Loading

Attempts to fix #7522

Added logs for each ignored exception. We tried to remove the exception but if we do so, we would need make changes to other codes as well and we're not sure whether it will affect the other codes. Hence, we just added LOGGER.warn, LOGGER.info or LOGGER.error for the ignored exceptions.

1 thing we added was line that creates a pop up window whenever the ImportException is reached in the ImportHandler.java, tryImportFormats method:
dialogService.notify(Localization.lang("ImportError: UnknownFormat"));

  • Change in CHANGELOG.md described in a way that is understandable for the average user (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 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.

@VernonKwa VernonKwa mentioned this pull request Oct 26, 2022
Closed
@Siedlerchr Siedlerchr added the type: code-quality Issues related to code or architecture decisions label Oct 27, 2022
@calixtus
Copy link
Member

calixtus commented Nov 8, 2022

Thanks four your PR. What's the status here? Is this ready for review?

@calixtus calixtus added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Nov 8, 2022
@ThiloteE ThiloteE added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: waiting-for-feedback The submitter or other users need to provide more information about the issue labels Nov 18, 2022
@ThiloteE
Copy link
Member

yes, there was a comment asking for review in the issue this pr will fix.

@ThiloteE
Copy link
Member

For next time: Use fix #... instead of fix issue #..., otherwise GitHub actions will not automatically close the issue.

@calixtus calixtus marked this pull request as ready for review November 19, 2022 21:05
koppor
koppor requested changes Dec 5, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Some minor comments. Please also fix merge conflicts.

src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Show resolved Hide resolved
src/main/java/org/jabref/logic/remote/server/RemoteListenerServer.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/util/BuildInfo.java Outdated Show resolved Hide resolved
@ThiloteE ThiloteE added status: changes required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Dec 7, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into Add-log-for-ignored…
fcea58d 
…-excepton

* upstream/main: (184 commits)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  Only arm64 m+
  Update deployment-arm64.yml
  add deployment with arm as well
  Revert "Test self hosted runner"
  remove self hosted from normal deployment
  fix unit test for slr
  Refine do not put under drive letter
  Update journal abbrev list (JabRef#9469)
  ...

# Conflicts:
#	src/main/java/org/jabref/logic/remote/server/RemoteListenerServer.java
@Siedlerchr
adust mostly to debug level as messages are not relevant for user
09045dd
@Siedlerchr Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: changes required Pull requests that are not yet complete labels Dec 19, 2022
@Siedlerchr Siedlerchr merged commit 62dbf0d into JabRef:main Dec 19, 2022
Siedlerchr added a commit to rafatd/jabref that referenced this pull request Dec 20, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into enable_drag'n'drop_…
0daff49 
…of_groups_to_new_library

* upstream/main: (148 commits)
  Fix remember last open valid library with empty new one (JabRef#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485)
  New Crowdin updates (JabRef#9483)
  Add log for ignored excepton (JabRef#9302)
  Select Library to import into (JabRef#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  Only arm64 m+
  Update deployment-arm64.yml
  ...
Siedlerchr added a commit to shafinkamal/jabref that referenced this pull request Dec 21, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-issue-8729
f95130c 
* upstream/main: (145 commits)
  Enable groups drag'n'drop to new library (JabRef#9460)
  Update MacOS jabrefHost.py to find local installs (JabRef#9487)
  Fix remember last open valid library with empty new one (JabRef#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485)
  New Crowdin updates (JabRef#9483)
  Add log for ignored excepton (JabRef#9302)
  Select Library to import into (JabRef#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  ...

# Conflicts:
#	CHANGELOG.md
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
Siedlerchr added a commit to Freeman6475/jabref that referenced this pull request Dec 22, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-issue-JabRef#6448
06aa986 
* upstream/main: (120 commits)
  Enable groups drag'n'drop to new library (JabRef#9460)
  Update MacOS jabrefHost.py to find local installs (JabRef#9487)
  Fix remember last open valid library with empty new one (JabRef#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485)
  New Crowdin updates (JabRef#9483)
  Add log for ignored excepton (JabRef#9302)
  Select Library to import into (JabRef#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479)
  Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480)
  Bibtex month not deprecated (JabRef#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  Also trigger on branch arm64mac-release
  ...
Siedlerchr added a commit that referenced this pull request Dec 25, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into update_jdk
c6dcead 
* upstream/main: (75 commits)
  Observable Preferences S (LastExportPath and Cleanups in JabRefPreferences and Globals) (#9493)
  Enable groups drag'n'drop to new library (#9460)
  Update MacOS jabrefHost.py to find local installs (#9487)
  Fix remember last open valid library with empty new one (#9489)
  Observable Preferences R (CitationKeyPatternPreferences) (#9486)
  Fixed ZBMathTest and extracted keyWordSeparator (#9485)
  New Crowdin updates (#9483)
  Add log for ignored excepton (#9302)
  Select Library to import into (#9402)
  Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (#9476)
  Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (#9478)
  Bump mockito-core from 4.9.0 to 4.10.0 (#9479)
  Bump checkstyle from 10.4 to 10.5.0 (#9477)
  Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (#9480)
  Bibtex month not deprecated (#9404)
  Show development information\n\n+semver: major
  Release v5.8
  Update external libraries add afterburner fx jabref add jakarta inject
  fix display name for artifact store
  Prepare CHANGELOG for release
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koppor koppor koppor requested changes

Assignees
No one assigned
Labels
Logging status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not ignore exception
6 participants
@VernonKwa @calixtus @ThiloteE @koppor @Siedlerchr @Yuxin0828