Update internal state of DatabaseChangeMonitor when external changes …#3503
Merged
Conversation
tobiasdiez
approved these changes
Dec 10, 2017
tobiasdiez
left a comment
Member
There was a problem hiding this comment.
Thanks for the fix! It seems to work fine on my side and the code looks also good to me (one small suggestion through).
|
|
||
| public void updateTimeStamp() { | ||
| changeMonitor.ifPresent(DatabaseChangeMonitor::markAsSaved); | ||
| changeMonitor.ifPresent(DatabaseChangeMonitor::updateTimestampAndFileSize); |
Member
There was a problem hiding this comment.
I would actually prefer to rename updateTimeStamp() to markAsSaved. That the DatabaseChangeMonitor uses the timestamp and file size to keep track of the last saved state, should be an implementation detail that is hidden to the outside world.
Member
Author
There was a problem hiding this comment.
Alright, I changed the name back to what it was before.
# Conflicts: # CHANGELOG.md
Member
Author
|
It seems we're having a bit of a shortage of reviewers at the moment ;) Since this is such a small one-line change that actually fixes a bug, I'll merge this now even if there's only one review. |
Siedlerchr
added a commit
that referenced
this pull request
Dec 13, 2017
* upstream/master: (108 commits) Fetcher for IACR eprints (#3473) Update internal state of DatabaseChangeMonitor when external changes … (#3503) Fixes #3505: Another try to fix the NPE in the search bar (#3512) Replace ' with ' so that our HTML preview can handle it correctly Added a "Clear text" button in right click menu within the text boxes. (#3475) Add reset to English language after a test New translations JabRef_en.properties (German) Remove ampersand in non-menu localizations New translations JabRef_en.properties (German) New translations Menu_en.properties (German) New translations Menu_en.properties (German) New translations JabRef_en.properties (Vietnamese) New translations JabRef_en.properties (Italian) New translations Menu_en.properties (Italian) New translations JabRef_en.properties (Indonesian) New translations Menu_en.properties (Indonesian) New translations JabRef_en.properties (Greek) New translations Menu_en.properties (Greek) New translations Menu_en.properties (Japanese) New translations JabRef_en.properties (German) ... # Conflicts: # build.gradle
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…are resolved
Attempts to fix #3498
It seems that the internal state of DatabaseChangeMonitor was not updated correctly when external changes were marked as resolved, causing the monitor to misbehave when you tried to save afterwards. I am not sure how this bug came into being and what caused the update to get lost. Adding an additional update step seems to resolve the problem, but I would really appreciate some more testing of this branch by other people.
gradle localizationUpdate?