[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Any exceptions for the 15-line rule?
From: |
Dmitry Gutov |
Subject: |
Re: Any exceptions for the 15-line rule? |
Date: |
Sat, 27 Apr 2013 16:26:12 +0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (windows-nt) |
Stefan Monnier <address@hidden> writes:
>> The latter is licensed under GPLv3+, but I'm pretty sure the author
>> (brianjcj AT gmail, not sure what's his full name) has not signed the
>> CA. And I'm not wild about the idea of waiting several months to add the
>> feature (that is, if the author even agrees to sign the CA).
>
> The change includes
>
> - (with-temp-buffer
> + (buf (get-buffer-create "*clang-output*"))
> + (with-current-buffer buf (erase-buffer))
> + (with-current-buffer buf
>
> Which seems like it's making the code worse rather than better.
> If you undo this undesirable part of the patch, it'll be closer to
> the acceptable limit.
Yes, I more or less reverted this piece in my mind already. :)
> For company-clang--lang-option, I'd be tempted to use
>
> (defun company-clang--lang-option ()
> (if (eq major-mode 'objc-mode)
> (if (string= "m" (file-name-extension buffer-file-name))
> "objective-c" "objective-c++")
> (substring (symbol-name major-mode) 0 -5)))
Thanks, `substring' is better than `replace-match' I mentioned. But
still, should this be considered a full new implementation? Does
replacing `cond' with `if' in the inner condition make it a new piece of
code, as opposed to derivative one?
> With such cleanups, the patch seems acceptable as a "tiny change".
> But please do ask for the CA as well (so the use of "tiny change" is
> mostly a way to avoid having to wait for the CA to go through).
To be clear, who do I ask to sign the CA over the modified patch? The
auto-complete-clang author, or the person who looked at a few pieces
from that package and adapted them to (admittedly, fairly similar)
company-clang code?