Skip to content

Issue 1501: Close media database after importing.#76

Merged
flerda merged 1 commit intoankidroid:v2.1-devfrom
hssm:Issue1501
Jul 7, 2013
Merged

Issue 1501: Close media database after importing.#76
flerda merged 1 commit intoankidroid:v2.1-devfrom
hssm:Issue1501

Conversation

@hssm
Copy link
Member

@hssm hssm commented Jul 7, 2013

Issue 1501 again. It was going to haunt me until I figured it out.

mSrc is the collection we are importing. Notice, however, that the collection itself isn't closed in the normal way. We're directly closing its database. This, however, means that the media database in the collection is still left open.

To be honest, I don't understand why we aren't closing it with Collection.close. Why do we need to need to keep the "count" (which is actually in AnkiDatabaseManager, contrary to the comment) of this database? Haven't we finished using it? I'm not sure what the implications of changing this are, so I've left it as-in and just closed the media database as well later down when we've finished importing media.

Copy link
Member

Choose a reason for hiding this comment

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

Is this intentional? It undoes your previous change.

Copy link
Member

Choose a reason for hiding this comment

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

I get it. This is meant to be the right fix, i.e., we no longer need to create multiple output directories, since now we class all the files in it, and delete will work!

Nice work!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that's exactly it.

@ghost ghost assigned flerda Jul 7, 2013
flerda added a commit that referenced this pull request Jul 7, 2013
Issue 1501: Close media database after importing.
@flerda flerda merged commit f976dfc into ankidroid:v2.1-dev Jul 7, 2013
@hssm hssm deleted the Issue1501 branch July 7, 2013 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants