Skip to content

issue 1501 and various improvements on unzipping code, importing#37

Merged
flerda merged 1 commit intoflerda:v2.0.1-devfrom
iniju:v2.0.1-import-fixes
Jan 30, 2013
Merged

issue 1501 and various improvements on unzipping code, importing#37
flerda merged 1 commit intoflerda:v2.0.1-devfrom
iniju:v2.0.1-import-fixes

Conversation

@iniju
Copy link
Copy Markdown

@iniju iniju commented Jan 27, 2013

progress messages, fixes on import media bugs

A general close inspection of the import code. There were multiple bugs on the media import (written earlier by me for 2.0.1, but never really tested, he, he), should be working better now, importing files when needed, skipping if the file already exists in collection.media folder and looks the same (only the first 1024 bytes are compared for speed reasons and of course only when filenames match).

Much better feedback to the user, as this operation can take a while.

@flerda
Copy link
Copy Markdown
Owner

flerda commented Jan 28, 2013

I have a few comments above. Let me know if they make sense and if you think we could update the code.

@iniju
Copy link
Copy Markdown
Author

iniju commented Jan 28, 2013

Code updated, summary of changes:

  • Removed unused unzip method.
  • Added somestream.close() in finally, but please have a look at it. I
    don't really like try/catch inside finally, even more so done thousands of
    times within a loop, so I put the whole for loop in a single try catch and
    made sure finally had close() calls in case we exit at a throw.
  • Added the column names in SQL calls.

Cheers,
Kostas

On Mon, Jan 28, 2013 at 11:05 PM, Flavio Lerda notifications@github.comwrote:

I have a few comments above. Let me know if they make sense and if you
think we could update the code.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-12807931.

@ghost ghost assigned flerda Jan 29, 2013
@flerda
Copy link
Copy Markdown
Owner

flerda commented Jan 29, 2013

I have a few more comments on this. Sorry for being a bit slow...

@iniju
Copy link
Copy Markdown
Author

iniju commented Jan 29, 2013

You're working very late! I'm at a training today, so I'll process them
later.

Thanks for the review
On Jan 29, 2013 6:20 AM, "Flavio Lerda" notifications@github.com wrote:

I have a few more comments on this. Sorry for being a bit slow...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-12821012.

flerda
flerda reviewed Jan 30, 2013
View reviewed changes
src/com/ichi2/libanki/Utils.java Outdated
Copy link
Copy Markdown
Owner

@flerda flerda Jan 30, 2013

Choose a reason for hiding this comment

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

This should also be FILE_COPY_BUFFER_SIZE.

@flerda
Copy link
Copy Markdown
Owner

flerda commented Jan 30, 2013

A couple of really tiny comments this time around.

@iniju
issue 1501 and various improvements on unzipping code, importing
8dfecf3 
progress messages, fixes on import media bugs - updated according to
feedback2
flerda added a commit that referenced this pull request Jan 30, 2013
@flerda
Merge pull request #37 from iniju/v2.0.1-import-fixes
0e51f48 
Import fixes.
@flerda flerda merged commit 0e51f48 into flerda:v2.0.1-dev Jan 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@flerda flerda

Labels

None yet

Projects

None yet

Milestone

2.0.1

Development

Successfully merging this pull request may close these issues.

2 participants

@iniju @flerda