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.

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

Add option to allow for integers in field "edition" when running database in bibtex mode #5121

Merged
merged 8 commits into from
Jul 15, 2019

Conversation

rodps
Copy link
Contributor

@rodps rodps commented Jul 13, 2019

Issue #4680

This is a new fork from the old pull request.

I added a checkbox in Preferences/GeneralTab to allow insert integers in field edition when bibtex mode is running. Also, i modified EditionChecker class to permit this configuration. As well as the test cases in IntegrityCheckTest class.


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Looks good to me from my point of view
@calixtus As you have been working on the Preferences tab it would be nice if you could look over it, if it's correct

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 13, 2019
@@ -78,6 +79,7 @@ public GeneralTabViewModel(DialogService dialogService, JabRefPreferences prefer
);
}

@Override
Copy link
Member

Choose a reason for hiding this comment

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

no need to override for setValues(), storeSettings() or validateSettings(), they directly implement the PrefsTab-Interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, this is because of the Eclipse IDE that put this automatically. I will correct this.

@calixtus
Copy link
Member

calixtus commented Jul 14, 2019

Ok, i focused on the preference-stuff, made a comment, everything else fits into the pattern, so i guess besides the "Override" - lgtm.
Did you notice the unresolved merge-conflicts?

@Siedlerchr
Copy link
Member

Please fix the changes proposed from @calixtus and merge the latest JabRef master branch in your branch

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks again for your contribution!

@Siedlerchr Siedlerchr merged commit e89d603 into JabRef:master Jul 15, 2019
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