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

Refactored exporter and split it into gui and logic #1574

Merged
merged 5 commits into from
Jul 16, 2016
Merged

Refactored exporter and split it into gui and logic #1574

merged 5 commits into from
Jul 16, 2016

Conversation

oscargus
Copy link
Contributor

Should be a no-brainer but better wait until after 3.5 with merging.

@oscargus oscargus added cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jul 11, 2016
@oscargus oscargus added this to the v3.6 milestone Jul 11, 2016
@oscargus
Copy link
Contributor Author

And probably after #1472 as well...

@oscargus oscargus force-pushed the exporterrefactoring branch 2 times, most recently from eae8ed7 to 441fd1c Compare July 11, 2016 22:53
@oscargus oscargus removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 11, 2016
lenhard
lenhard reviewed Jul 12, 2016
View reviewed changes
src/main/java/net/sf/jabref/gui/exporter/ExportFileFilter.java
@@ -13,26 +13,28 @@
with this program; if not, write to the Free Software Foundation, Inc.,
51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
package net.sf.jabref.exporter;
package net.sf.jabref.gui.exporter;
Copy link
Member

Choose a reason for hiding this comment

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

This class is really independent of GUI code, so it should be in logic and not in gui.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

import javax.swing.filechooser.FileFilter;

Had to restructure ExportFormat a bit because of that darn FileFilter...

Copy link
Member

Choose a reason for hiding this comment

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

I see. You are correct! That means the PR is fine from my point of view.

@lenhard
Copy link
Member

lenhard commented Jul 12, 2016

Nice work! I am looking into architectural structure at the moment as well. I would only move one class, as mentioned above.

I think it would be cool if we got the AutoSaveManager to work independent of the GUI, but that would mean a rewrite. Thus, it is out of the scope of this PR.

@oscargus
Copy link
Contributor Author

Thanks! The main purpose was really to see which classes are "easily" testable (=logic) at the moment.

@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 13, 2016
@stefan-kolb
Copy link
Member

LGTM 👍

@oscargus oscargus merged commit e8cc789 into JabRef:master Jul 16, 2016
@oscargus oscargus deleted the exporterrefactoring branch July 16, 2016 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cleanup-ops status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
v3.6
Development

Successfully merging this pull request may close these issues.
3 participants
@oscargus @lenhard @stefan-kolb