Code review comment for lp://qastaging/~dholbach/command-not-found/1479805

Revision history for this message
CarstenHey (c.hey) wrote :

Hi,

thanks for putting all this into a merge request :)

You copied my error into the bash part of the commit message, this:

| - Do not define the handler function if the shell is interactive.

should be:

| - Do not define the handler function if the shell is not interactive.

Also, this part is missing in the commit message (but the fix is
upstream only, to fix it in Ubuntu, the bash package has to be
adapted):

| - Don't run command-not-found if bash_completion is used.

Changing the merge request just because of an suboptimal commit
message (JFTR: this was my fault) might be a waste of time, though.

The reason I choose to abstain instead of to approve is because
the Ubuntu package already contains a fixed zsh_command_not_found,
to which this merge request adds some minor improvements. If merging
from the package to upstream first avoids a merge conflict, then
doing so might be a good idea - since I lack some essential
launchpad related knowledge, I'm not able to judge this.

The changelog entry of the previous upload to wily is:

  [ Carsten Hey ]
  * Update /etc/zsh_command_not_found:
    - Don't overwrite an already defined command-not-found handler.
    - Don't try to run command-not-found if the package has been
      removed since the shell has been started.

  [ Michael Vogt ]
  * updated for latest wily

 -- Michael Vogt <email address hidden> Tue, 21 Jul 2015 08:02:05 +0200

Even though the actual bug does not affect the Ubuntu package,
I think this patch should be applied - I just don't know which
way the former change to zsh_command_not_found should be merged
(actually, the new file should replace the old one, but bzr does
not know that).

Regards
Carsten

review: Abstain

« Back to merge proposal