Merge lp://staging/~aladin/deja-dup/imap-support into lp://staging/~deja-dup-team/deja-dup/trunk

Proposed by Lars J.
Status: Rejected
Rejected by: Michael Terry
Proposed branch: lp://staging/~aladin/deja-dup/imap-support
Merge into: lp://staging/~deja-dup-team/deja-dup/trunk
Diff against target: 672 lines (+419/-3) (has conflicts)
18 files modified
.bzrignore (+3/-0)
AUTHORS (+1/-0)
common/Backend.vala (+3/-0)
common/BackendIMAP.vala (+171/-0)
common/IMAP_Configuration_Helper.vala (+101/-0)
common/Makefile.am (+2/-0)
data/org.gnome.DejaDup.gschema.xml.in (+35/-0)
help/translations/deja-dup-help.pot (+4/-0)
po/POTFILES.in (+3/-0)
po/POTFILES.skip (+3/-0)
widgets/ConfigBool.vala (+1/-1)
widgets/ConfigEntry.vala (+2/-1)
widgets/ConfigLabelLocation.vala (+1/-0)
widgets/ConfigLocation.vala (+9/-0)
widgets/ConfigLocationIMAP.vala (+77/-0)
widgets/ConfigNumber.vala (+1/-1)
widgets/Makefile.am (+1/-0)
widgets/WidgetUtils.vala (+1/-0)
Text conflict in help/translations/deja-dup-help.pot
Contents conflict in po/deja-dup.pot
To merge this branch: bzr merge lp://staging/~aladin/deja-dup/imap-support
Reviewer Review Type Date Requested Status
Michael Terry Disapprove
Review via email: mp+46248@code.staging.launchpad.net

Description of the change

This branch implements imap support.

- Pay attention to Duplicity Bug #385495
- po/deja-dup.pot is removed from my branch. Why? I run make update-pot
- The list of known IMAP service providers is hard-coded in common/IMAP_Configuration_Helper.vala. In the future it is better to maintain the list in a extra file.

To post a comment you must log in.
Revision history for this message
Michael Terry (mterry) wrote :

This looks like a very nice patch. I've been in a conference and haven't had the chance to do a full review, including running it yet. I will soon.

However, do you know if this violates some terms of service with Google? I would find it surprising if they were A-OK with such usage...

Revision history for this message
Michael Terry (mterry) wrote :

Also, this may be easiest to expose as a list of specific providers. Like maybe offer GMail in a dropdown. That way, we'd not need to show a preference for max attachment size, server name, and whether to use https, and would know to use "labels" or "folders" or whatnot to describe where the user is putting things. Since we have a hardcoded list anyway....

If we ended up getting more IMAP providers, I wouldn't want to clog the main location dropdown, so this might be best done as a an entry like "IMAP (email)" in the main location dropdown and then a subdropdown below that saying which provider.

Revision history for this message
Lars J. (aladin) wrote :

This patch does not clog the main location dropdown; it adds one entry "Email service providing MAP access". After the user has entered his email-address the patch check if this address is matching one of the known IMAP service providers. If true then server_address, use_imaps are set and the corresponding widgets are deactivated.

Does this proposal violates some terms of service with Google?

Yes - big problem. See http://www.google.com/accounts/TOS
Section 5.3 You agree not to access (or attempt to access) any of the Services by any means other than through the interface that is provided by Google, unless you have been specifically allowed to do so in a separate agreement with Google. You specifically agree not to access (or attempt to access) any of the Services through any automated means (including use of scripts or web crawlers) and shall ensure that you comply with the instructions set out in any robots.txt file present on the Services.

I think that Deja-Dup should not entice its users to violate some terms of service with Google (and to face disabled accounts). I suggest to rewrite the imap support to present just a plain form with a warning against not to violate some terms of service.

Revision history for this message
Michael Terry (mterry) wrote :

You're right that it doesn't clog the main dropdown. I was talking out loud about if we were to offer a drop down for services, where that list of services should go (in main dropdown or sub dropdown).

I liked a dropdown because since the support of servers was hardcoded, it didn't make much sense to have user enter email address and then say that we had a secret list of servers we supported and their address didn't match it. Easier to just show them the servers we support and then we can know all the parameters for that server from there.

So since it violates TOS, I'm thinking we shouldn't add support for this at all if it can't be done in a user-friendly, well-integrated way.

I do like the patch, and maybe it can be resurrected if things change.

review: Disapprove

Unmerged revisions

784. By Lars J.

update pot files; change my email-address

783. By Lars J.

initial release of imap support

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches