Merge lp:~viktor-nagy/bzr-email/tip-hook into lp:bzr-email

[Jelmer Vernooij]

[Jelmer Vernooij (jelmer)]

Hi,

I'm sorry it's taken so long before we had a chance to look at this branch. Unfortunately it now has conflicts with trunk. If you still care about getting this functionality in, is there any chance you can resolve them and resubmit? Please don't hesitate to ask us if we don't follow up to a MP.

review: Needs Fixing (code)

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/17/2011 4:01 PM, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~viktor-nagy/bzr-email/tip-hook into lp:bzr-email.
>
> Requested reviews:
> Bazaar Developers (bzr)
>
> For more details, see:
> https://code.launchpad.net/~viktor-nagy/bzr-email/tip-hook/+merge/50238

This is conflicting with trunk after Jelmer's earlier update.

The way it is written it will default to smtp_use_tls = False, I would
rather default to True, but a user can disable it with an explicit
setting. (because get() defaults to returning None which evaluates to
False.)

I think just adding

use_tls = config.get_user_option('smtp_use_tls')
if use_tls is None:
  use_tls = True

...

On the other hand, isn't the connection side of things handled by bzr
code now? At least, I thought bzr-email was using bzrlib.smtp_connection
now. So that code should be pulled into bzr core.

I also don't fully understand what this is for:
+def tip_change_hook(params):
+ '''
+ This is the post_change_branch_tip commit hook that runs on a smart
server after the branch's tip has changed.
+ '''
+ from bzrlib.branch import BzrBranch6
+ if not isinstance(params.branch, BzrBranch6):
+ raise errors.UpgradeRequired(params.branch.base)
+ _emailer.EmailSender(params.branch, params.new_revid,
params.branch.get_config(),
+ local_branch=None).send_maybe()
+

Why do you have to have BzrBranch6 or better? Certainly that sounds like
something the user wanted to enforce, but should not be something in
bzr-email itself.)

 review: needsfixing

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk1dnoMACgkQJdeBCYSNAAOD7wCePJk/Qk40OwoaSMcmTgEN9Xdu
/GoAoKPWkL91NlhA+wXlETOi4aNcKdEi
=M3nm
-----END PGP SIGNATURE-----

review: Needs Fixing

Unmerged revisions

38. By ViktorNagy on 2008-08-21

Change the name for the post change branch tip hook from tip_commit to
tip_change

37. By ViktorNagy on 2008-08-18

* added post_change_branch_tip hook
* adds a new config option smtp_use_ttls = True | False, and corrects
the statttls() negotiation

=== modified file '__init__.py'
--- __init__.py	2010-10-09 23:15:45 +0000
+++ __init__.py	2011-02-17 22:01:52 +0000
@@ -79,6 +79,7 @@
 # lazy_import emailer so that it doesn't get loaded if it isn't used
 lazy_import(globals(), """\
 from bzrlib.plugins.email import emailer as _emailer
+from bzrlib.branch import BzrBranch6
 """)
 
 
@@ -94,6 +95,16 @@
     _emailer.EmailSender(master, new_revid, master.get_config(),
                          local_branch=local).send_maybe()
 
+def tip_change_hook(params):
+    '''
+    This is the post_change_branch_tip commit hook that runs on a smart server after the branch's tip has changed.
+    '''
+    from bzrlib.branch import BzrBranch6
+    if not isinstance(params.branch, BzrBranch6):
+        raise errors.UpgradeRequired(params.branch.base)
+    _emailer.EmailSender(params.branch, params.new_revid, params.branch.get_config(),
+                         local_branch=None).send_maybe()
+    
 
 def branch_post_change_hook(params):
     """This is the post_change_branch_tip hook."""
@@ -107,9 +118,13 @@
     install_named_hook = getattr(Branch.hooks, 'install_named_hook', None)
     if install_named_hook is not None:
         install_named_hook('post_commit', branch_commit_hook, 'bzr-email')
+<<<<<<< TREE
         if 'post_change_branch_tip' in Branch.hooks:
             install_named_hook('post_change_branch_tip',
                 branch_post_change_hook, 'bzr-email')
+=======
+        install_named_hook('post_change_branch_tip', tip_change_hook, 'bzr-email-on-tip')
+>>>>>>> MERGE-SOURCE
     else:
         Branch.hooks.install_hook('post_commit', branch_commit_hook)
         if getattr(Branch.hooks, 'name_hook', None) is not None:

=== modified file 'smtp_connection.py'
--- smtp_connection.py	2011-02-14 14:22:40 +0000
+++ smtp_connection.py	2011-02-17 22:01:52 +0000
@@ -55,6 +55,7 @@
 
         self._smtp_username = config.get_user_option('smtp_username')
         self._smtp_password = config.get_user_option('smtp_password')
+        self._use_ttls = config.get_user_option('smtp_use_ttls')
 
         self._connection = None
 
@@ -85,6 +86,7 @@
         # If this fails, it just returns an error, but it shouldn't raise an
         # exception unless something goes really wrong (in which case we want
         # to fail anyway).
+<<<<<<< TREE
         try:
             self._connection.starttls()
         except smtplib.SMTPException, e:
@@ -95,6 +97,12 @@
                 pass
             else:
                 raise
+=======
+        if self._use_ttls:
+            self._connection.ehlo()
+            self._connection.starttls()
+            self._connection.ehlo()
+>>>>>>> MERGE-SOURCE
 
     def _authenticate(self):
         """If necessary authenticate yourself to the server."""