[gpodder] Re: device sync: per-channel settings

  • From: Thomas Perl <th.perl@xxxxxxxxx>
  • To: Rafi Rubin <rafi@xxxxxxxxxxxxxxxx>
  • Date: Wed, 5 Sep 2012 11:18:05 +0200

Hi Rafi,

2012/8/30 Rafi Rubin <rafi@xxxxxxxxxxxxxxxx>:
> On 08/30/12 02:58, Thomas Perl wrote:
>> As you can't insert columns in SQLite between old columns (see
>> http://www.sqlite.org/lang_altertable.html) without having to resort to
>> temporary tables and copying, add the new column at the end of the list of
>> columns, so that users upgrading from old database versions and users
>> installing gPodder afresh have the same ordering of columns. Also, what
>> about naming the column "sync_enabled", defaulting to 1 (INTEGER NOT NULL
>> DEFAULT 1)?
>
> Guess I'm spoiled from using nicer dbs.  Does the update function copy and
> rebuild the table, or are you suggesting that I put the field at the end of
> the list?  Does column order matter in this case?  Is it just cosmetic or are
> there portions of the code that actually depend on field order in records (I
> know that can go either way depending on certain sql syntax).

Right now, it only appends new columns at the end, we could
theoretically do a copy-and-rebuild, but I guess it's easier and
faster to just append columns at the end. Putting the field at the end
of the list is probably easiest for this reason. The column order
shouldn't matter for the most parts, but just to be safe, let's keep
the ordering stable when possible.

> Looks like the old field name was "sync_to_devices".  Do you prefer that or
> your suggested "sync_enabled"?

"sync_enabled" sounds good to me, choose which one you like best.

>> PS: Will you be submitting a merge request for
>> https://github.com/rafiyr/gpodder/commit/d91b837880? :)
>
> Thought I already did.  Should I try again?

No, it worked, and I merged it today. Sorry for the delay ;)


Thanks,
Thomas

Other related posts: