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

NPE when exporting the database to an .sql file #778

Merged
merged 3 commits into from
Feb 10, 2016
Merged

NPE when exporting the database to an .sql file #778

merged 3 commits into from
Feb 10, 2016

Conversation

jensdoecke
Copy link
Contributor

Couldn't resolve the root cause, because of a mixup between Connection and Filehandling (disregards SoC)

@oscargus
Copy link
Contributor

oscargus commented Feb 9, 2016

Thanks for the contribution. Makes sense!

Next time, please create a branch for your PR. I think that there may be some problems merging from your master branch so I let someone more knowledgeable do it.

@oscargus oscargus added [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Feb 9, 2016
@simonharrer
Copy link
Contributor

Could you add a CHANGELOG entry, please?

For us, it is not soo much an issue if this is on the master branch. So when a CHANGELOG entry is added, we can merge this in.

But maybe we should refactor the code to avoid these if statements whether we have a database connection or a file handle.

@jensdoecke
Copy link
Contributor Author

I added the Fix to the CHANGELOG.
These are my first steps in open source contribution so please forgive me the beginnner faults. ;-)

@tobiasdiez
Copy link
Member

Thanks @jensdoecke.
Can you please shortly explain why you tested that out is a connection and not that val is not null?

@jensdoecke
Copy link
Contributor Author

In my opinion is the code more readable.

I prefer something like this:
When the object is a connection, I want to check if it is a MySql DB.

Instead of the assumption:
When the object is null, it must be a FileOutputStream and no Connection, so I don't check the dbParams.

@tobiasdiez
Copy link
Member

Ok thanks for this clarification and your pullrequest. In my opinion, this can be merged in. 👍

@stefan-kolb
Copy link
Member

Thanks for the PR 👍

stefan-kolb added a commit that referenced this pull request Feb 10, 2016
NPE when exporting the database to an .sql file
@stefan-kolb stefan-kolb merged commit a243769 into JabRef:master Feb 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[outdated] type: bug Confirmed bugs or reports that are very likely to be bugs 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.

5 participants