Re: Duplicates patch new version - Check box in importing

[Alvaro del Castillo <acs openshine com>]

Hi!

El dom, 30-10-2005 a las 01:52 -0500, Larry Ewing escribió:
> This message is getting long so I'll delete long chunks.
> 

Great! I will also cut all the agreed so we can concentrate on the final
patch work.

> > Done! Now only 1 white space between the type and the variable name and
> > all the lines using 8 space tabs.
> > 
> 
> Thanks, I know it is a bit pedantic but it will help in the long run.
> 

I really like to have a clear coding style so we can look to the code in
5-10 years with thousands of line of code and see how beautiful it is :)

> > But I feel the same as you here. We should migrate the all database
> > format to the new format and from now, change the README file if we plan
> > to support the current database scheme so users could start investing
> > time in filling their photos database.
> > 
> > I see here a clear stopper for the patch until this upgrade code is
> > done, no? I will do it but don't know exactly the time frame.
> > 
> > 
> 
> Yeah, it needs to at least do something sane when it encounters an old
> db.
> 

The sane thing I think it is to add the new MD5 field with empty values
for the old photos. I will code this.

> 
> This looks good, I wonder if there are circumstances where we'd want to
> actually get the duplicate photos? 
> 

If the user says she doesn't want duplicates I think it is sure to drop
them ... I can't think in a use case where we want to import duplicates
if the user doesn't want them.

> [snip]
> 
> > > This contains another implementation of the PhotoMD5 method, use the one
> > > you already have above (once it is fixed).  Also this might be a
> > > candidate for using the query as well, at least in the nothing is
> > > selected case.
> > 
> > Yes, this is the first implementation I have done when we aren't
> > creating the MD5Sum in import. I think this should be cleaned out if the
> > MD5Sum is computed in importing so all this code is removed right now in
> > my CVS.
> > 
> 
> Great, that makes sense.

Great, I will clean that code for now. We lost the feature to clean
already imported duplicates or for example, to compute the MD5Sum for
all the photo don't have it. But I think that the correct thing is that
in the upgrading code for the database when converting, compute the MD5
for all the photos doesn't have them. So I will use some of this code in
the upgrading database process.

> The tag seems a bit confusing too me and extends the meaning of tags in
> a way I'm not sure we need.  Let's not tag them for now, and just use
> the menu item.  If we come up with a compelling use for the tag we can
> always add it later.
> 

Ok, about the menu item, I am not sure we want it. If we don't use the
duplicate tag, we need to find some natural way to show the duplicates
to the user. And I am not sure we want that. Maybe just a menu entry
with "Clean duplicates" is better. 


> At least to the point of not crashing with old databases.
> 

So we can inform the use about the issue and guide her to remove the old
database. But as I said, I am trying to upgrade the database.

> > 2. The MD5Sum should be stored in memory as an array bit and converted
> > when reading as a string from the database and writing as a string to
> > the database. Right?
> 
> Sounds good.
> 
> > 3. IsDuplicate right now is moved to PhotoStore.cs and queries the
> > database to find it the to be imported photo is a duplicate. Right?
> > 
> 
> yes.
> 
> > 4. We want duplicate photo detection after importing? If no I will clean
> > all the code about it. If yes, should we continue using the tag
> > duplicate? If maybe, I will clean the code in order to include the patch
> > upstream for importing and we should continue talking about it.
> > 
> 
> Let's keep duplicate detection after import menu item but not use the
> tag for now, unless you have have a strong reason for it.

I don't have a strong reason for it. I am starting to have strong
reasons about it as more as I think in it. I think the "Clear
duplicates" menu entry could be right way to go. The user doesn't want
to find duplicates, she wants to remove them or keep them.

> 
> > 
> > Ok, I think that's all folks! Now time to make some decissions and I
> > will send an updated patch to the list.
> > 
> 
> Thanks again Alvaro, I really appreciate all the work.

Ok, I think I have all the information to finish the patch. Sorry for
introducing the "Clear duplicates" issue now, but I think it is the way
to go. 

I hope I'll find time this week to work on the patch and resend it to
the list.

Cheers 

-- Alvaro

> 
> --Larry
> 
> 
>