unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] What's the quickest way to contribute?
@ 2015-01-08 17:59 Oleh Krehel
  2015-01-08 18:48 ` Eli Zaretskii
  2015-01-13  2:57 ` Dmitry Gutov
  0 siblings, 2 replies; 11+ messages in thread
From: Oleh Krehel @ 2015-01-08 17:59 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2374 bytes --]

Hi all,

I like Emacs a lot and I'd like to contribute to the development.  I
have assigned the FSF Copyright and made some small contributions in
the past, but so far my contributions have been moving slower than
molasses.

For instance, this one-liner,
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19493, has been hanging
there for a few days now.

The other one, https://debbugs.gnu.org/cgi/bugreport.cgi?bug=15920, is
more than a year old.

This one, https://debbugs.gnu.org/cgi/bugreport.cgi?bug=19460, Stefan
himself asked me to report, and yet there's no answer so far.

A few questions/ possible answers follow on why this is:

1. My patches / bug reports are lame. Then I'd appreciate at least
some feedback pointing out the faults. I learn quickly.

2. I'm submitting to the wrong place, e.g. to the bug tracker instead
of the devel list or vice versa. I'd like to know the proper and
approved way to do this.

3. My patches / bug reports are low priority and are not worth
applying / responding to. Then please let me know the priority areas
in which contributions are more actively accepted.

4. The devs are swamped with bug reports and I should just wait a few
months more until my turn. Well, I could help out in that area, since
my intention is to start from small contributions into larger ones
until I get experienced enough to get push access and get my work
merged without a lengthy review. I could then help with the review of
other people's one-liners.

I've read http://lars.ingebrigtsen.no/2014/11/13/welcome-new-emacs-developers/
and http://www.emacswiki.org/emacs/GitForEmacsDevs, but I didn't find
the answers to the questions that I'm posting here.

Finally, I attach one more patch that I'd like some feedback on.
I was browsing replace.el and found this type of code:

    (delq nil (mapcar #'(lambda (buf)
                          (when (buffer-live-p buf) buf))
                      bufs))

instead of this:

    (cl-remove-if-not #'buffer-live-p bufs)

If this were my package, I would change it without batting an eye,
since the latter code is more clear and algorithmically faster.
However, maybe my knowledge of Emacs's internals is lacking and the
current way is better.  Or maybe there's a policy against `cl-`
functions. Or maybe there's a policy of not fixing things that aren't
broke. I'd like to know these things.

regards,
Oleh

[-- Attachment #2: 0001-lisp-replace.el-occur-1-Use-cl-seq.patch --]
[-- Type: text/x-patch, Size: 1338 bytes --]

From 1da7d6fb880834772ad17aed46872d49641e1f6b Mon Sep 17 00:00:00 2001
From: Oleh Krehel <ohwoeowho@gmail.com>
Date: Thu, 8 Jan 2015 17:40:59 +0100
Subject: [PATCH] lisp/replace.el (occur-1): Use `cl-seq'.

* lisp/replace.el (occur-1): Use `cl-remove-if-not' and `cl-find-if'
  in favor of more complex operations.
---
 lisp/replace.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lisp/replace.el b/lisp/replace.el
index e0636e0..b92e3cd 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1416,14 +1416,15 @@ See also `multi-occur'."
   (unless buf-name
     (setq buf-name "*Occur*"))
   (let (occur-buf
-	(active-bufs (delq nil (mapcar #'(lambda (buf)
-					   (when (buffer-live-p buf) buf))
-				       bufs))))
+	(active-bufs
+         (cl-remove-if-not #'buffer-live-p bufs)))
     ;; Handle the case where one of the buffers we're searching is the
     ;; output buffer.  Just rename it.
-    (when (member buf-name (mapcar 'buffer-name active-bufs))
+    (when (cl-find-if
+           `(lambda (buf) (equal (buffer-name buf) ,buf-name))
+           active-bufs)
       (with-current-buffer (get-buffer buf-name)
-	(rename-uniquely)))
+        (rename-uniquely)))
 
     ;; Now find or create the output buffer.
     ;; If we just renamed that buffer, we will make a new one here.
-- 
1.8.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-08 17:59 [PATCH] What's the quickest way to contribute? Oleh Krehel
@ 2015-01-08 18:48 ` Eli Zaretskii
  2015-01-08 19:04   ` Samer Masterson
  2015-01-13  2:57 ` Dmitry Gutov
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2015-01-08 18:48 UTC (permalink / raw)
  To: Oleh Krehel; +Cc: emacs-devel

> Date: Thu, 8 Jan 2015 18:59:09 +0100
> From: Oleh Krehel <o.krehel@tue.nl>
> 
> I like Emacs a lot and I'd like to contribute to the development.  I
> have assigned the FSF Copyright and made some small contributions in
> the past, but so far my contributions have been moving slower than
> molasses.

Thanks for your contributions, and sorry for being so slow in reacting
on them.

> A few questions/ possible answers follow on why this is:
> 
> 1. My patches / bug reports are lame. Then I'd appreciate at least
> some feedback pointing out the faults. I learn quickly.
> 
> 2. I'm submitting to the wrong place, e.g. to the bug tracker instead
> of the devel list or vice versa. I'd like to know the proper and
> approved way to do this.
> 
> 3. My patches / bug reports are low priority and are not worth
> applying / responding to. Then please let me know the priority areas
> in which contributions are more actively accepted.
> 
> 4. The devs are swamped with bug reports and I should just wait a few
> months more until my turn.

I wouldn't say "swamped", but definitely busy.  So:

5. Please ping after a week that you didn't see any replies for your
report, and keep pinging until things move.

> I could then help with the review of other people's one-liners.

Please do, and thanks in advance.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-08 18:48 ` Eli Zaretskii
@ 2015-01-08 19:04   ` Samer Masterson
  2015-01-08 19:13     ` Dmitry Gutov
  2015-01-09  0:01     ` Richard Stallman
  0 siblings, 2 replies; 11+ messages in thread
From: Samer Masterson @ 2015-01-08 19:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Oleh Krehel, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]

On the topic of new developers, does anybody know how long the copyright
assignment process is supposed to take? I signed the copyright assignment
papers three weeks ago, and I have a couple patches
sitting in debbugs that are waiting for my copyright assignment to be fully
processed. And it took me a week to receive the copyright assignment form
after I sent my application, which means that I've waited an entire month
for my patch to be accepted into Emacs.

I know encouraging new developers has been a popular topic on the list for
the past couple months. But we probably shouldn't get ahead of ourselves if
it takes weeks for a new contributor to get anything merged in :)

Best,
Samer

On Thu, Jan 8, 2015 at 10:48 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Thu, 8 Jan 2015 18:59:09 +0100
> > From: Oleh Krehel <o.krehel@tue.nl>
> >
> > I like Emacs a lot and I'd like to contribute to the development.  I
> > have assigned the FSF Copyright and made some small contributions in
> > the past, but so far my contributions have been moving slower than
> > molasses.
>
> Thanks for your contributions, and sorry for being so slow in reacting
> on them.
>
> > A few questions/ possible answers follow on why this is:
> >
> > 1. My patches / bug reports are lame. Then I'd appreciate at least
> > some feedback pointing out the faults. I learn quickly.
> >
> > 2. I'm submitting to the wrong place, e.g. to the bug tracker instead
> > of the devel list or vice versa. I'd like to know the proper and
> > approved way to do this.
> >
> > 3. My patches / bug reports are low priority and are not worth
> > applying / responding to. Then please let me know the priority areas
> > in which contributions are more actively accepted.
> >
> > 4. The devs are swamped with bug reports and I should just wait a few
> > months more until my turn.
>
> I wouldn't say "swamped", but definitely busy.  So:
>
> 5. Please ping after a week that you didn't see any replies for your
> report, and keep pinging until things move.
>
> > I could then help with the review of other people's one-liners.
>
> Please do, and thanks in advance.
>
>

[-- Attachment #2: Type: text/html, Size: 2978 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-08 19:04   ` Samer Masterson
@ 2015-01-08 19:13     ` Dmitry Gutov
  2015-01-09  0:01     ` Richard Stallman
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2015-01-08 19:13 UTC (permalink / raw)
  To: Samer Masterson, Eli Zaretskii; +Cc: Oleh Krehel, emacs-devel

On 01/08/2015 10:04 PM, Samer Masterson wrote:
> On the topic of new developers, does anybody know how long the copyright
> assignment process is supposed to take? I signed the copyright
> assignment papers three weeks ago, and I have a couple patches
> sitting in debbugs that are waiting for my copyright assignment to be
> fully processed. And it took me a week to receive the copyright
> assignment form after I sent my application, which means that I've
> waited an entire month for my patch to be accepted into Emacs.

That's quite normal, unfortunately. On the bright side, US and German 
contributors can sign the assignment electronically, which happens much 
faster.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-08 19:04   ` Samer Masterson
  2015-01-08 19:13     ` Dmitry Gutov
@ 2015-01-09  0:01     ` Richard Stallman
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Stallman @ 2015-01-09  0:01 UTC (permalink / raw)
  To: Samer Masterson; +Cc: eliz, o.krehel, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

The FSF staff person was probably on vacation for a week or so.
I suggest you email assign@gnu now asking where things stand.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-08 17:59 [PATCH] What's the quickest way to contribute? Oleh Krehel
  2015-01-08 18:48 ` Eli Zaretskii
@ 2015-01-13  2:57 ` Dmitry Gutov
  2015-01-13  5:10   ` Stefan Monnier
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2015-01-13  2:57 UTC (permalink / raw)
  To: Oleh Krehel, emacs-devel

On 01/08/2015 08:59 PM, Oleh Krehel wrote:

Regarding this patch:

> Finally, I attach one more patch that I'd like some feedback on.
> I was browsing replace.el and found this type of code:
>
>      (delq nil (mapcar #'(lambda (buf)
>                            (when (buffer-live-p buf) buf))
>                        bufs))
>
> instead of this:
>
>      (cl-remove-if-not #'buffer-live-p bufs)
>
> If this were my package, I would change it without batting an eye,
> since the latter code is more clear and algorithmically faster.

Personally, I'd hesitate to make a change like this unless there's a 
semi-plausible scenario where this code is wrong (when a `buf' values 
could be nil), or where the change in performance could plausibly be 
noticed by a user. Since the number of buffers opened in Emacs is very 
unlikely to be high enough, probably not.

And the code reads well enough.

 > Or maybe there's a policy of not fixing things that aren't broke.

Mostly this.

Using `cl-' functions is allowed, but I guess there remains some amount 
of prejudice against the CL coding style, and using those (more 
complicated) functions when simpler ones can work just as well.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-13  2:57 ` Dmitry Gutov
@ 2015-01-13  5:10   ` Stefan Monnier
  2015-01-13 11:15     ` Dmitry Gutov
  2015-01-13 13:46     ` Oleh Krehel
  0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier @ 2015-01-13  5:10 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Oleh Krehel, emacs-devel

>> Finally, I attach one more patch that I'd like some feedback on.
>> I was browsing replace.el and found this type of code:
>>
>> (delq nil (mapcar #'(lambda (buf)
>>                       (when (buffer-live-p buf) buf))
>>                   bufs))
>>
>> instead of this:
>>
>> (cl-remove-if-not #'buffer-live-p bufs)
>>
>> If this were my package, I would change it without batting an eye,
>> since the latter code is more clear and algorithmically faster.

I think algorithmically, it's equivalent (both are O(N), basically).
I wouldn't be able to predict which one will be faster, but my guess is
that it'll be a wash (and I'd expect the `delq' loop to be negligible
compared to the `mapcar' loop).

So the clarity aspect is the more important argument.

> Using `cl-' functions is allowed, but I guess there remains some amount of
> prejudice against the CL coding style, and using those (more complicated)
> functions when simpler ones can work just as well.

Yes, cl-* functions are definitely allowed.  There is of course a lot of
carried prejudice from when cl-* functions didn't exist (and we just
had the `remove-if-not' instead, whose use was not accepted in Emacs's
own code), but there is also still a restriction in this respect: cl-*
functions still can't be used from preloaded files (because that would
require preloading cl-lib).


        Stefan



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-13  5:10   ` Stefan Monnier
@ 2015-01-13 11:15     ` Dmitry Gutov
  2015-01-13 19:24       ` Stefan Monnier
  2015-01-13 13:46     ` Oleh Krehel
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2015-01-13 11:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Oleh Krehel, emacs-devel

On 01/13/2015 08:10 AM, Stefan Monnier wrote:

 > I think algorithmically, it's equivalent (both are O(N), basically).

On some inputs, `cl-remove-if-not' could save on memory churn (less 
consing in the process), if the result list is small. Or if there's 
nothing to remove: it will simply return the original list.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-13  5:10   ` Stefan Monnier
  2015-01-13 11:15     ` Dmitry Gutov
@ 2015-01-13 13:46     ` Oleh Krehel
  2015-01-13 19:32       ` Stefan Monnier
  1 sibling, 1 reply; 11+ messages in thread
From: Oleh Krehel @ 2015-01-13 13:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

> I think algorithmically, it's equivalent (both are O(N), basically).

Yes, I've used my blurry version of *algorithmically faster*, which
means iterating on one list instead of two in succession; the
complexity is the same, of course.

> Yes, cl-* functions are definitely allowed.  There is of course a lot of
> carried prejudice from when cl-* functions didn't exist (and we just
> had the `remove-if-not' instead, whose use was not accepted in Emacs's
> own code), but there is also still a restriction in this respect: cl-*
> functions still can't be used from preloaded files (because that would
> require preloading cl-lib).

By preloaded files, do you mean the ones on `preloaded-file-list'?
There are 112 files in this list on my system, so it's quite a large
restriction. Is it no-more, no-less, i.e. only these 112 files?

Maybe the byte compiler could do some linting to enforce this
restriction?  It already contains a lot of checks.

Oleh



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-13 11:15     ` Dmitry Gutov
@ 2015-01-13 19:24       ` Stefan Monnier
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2015-01-13 19:24 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Oleh Krehel, emacs-devel

>> I think algorithmically, it's equivalent (both are O(N), basically).
> On some inputs, `cl-remove-if-not' could save on memory churn (less consing
> in the process), if the result list is small. Or if there's nothing to
> remove: it will simply return the original list.

Yes, they don't have quite the same efficiency both in terms of number
of instructions and memory use.
Still makes no difference "algorithmically" (i.e. still O(N)).

And my guess is still that in practice it will be a wash (even for
specially crafted benchmarks, I'd expect the performance to be very
similar, based on my past experience trying to use compiler-macros to
turn mapcar into dolist).


        Stefan



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] What's the quickest way to contribute?
  2015-01-13 13:46     ` Oleh Krehel
@ 2015-01-13 19:32       ` Stefan Monnier
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Monnier @ 2015-01-13 19:32 UTC (permalink / raw)
  To: Oleh Krehel; +Cc: emacs-devel

>> I think algorithmically, it's equivalent (both are O(N), basically).
> Yes, I've used my blurry version of *algorithmically faster*, which
> means iterating on one list instead of two in succession; the
> complexity is the same, of course.

As mentioned, I expect the second traversal (the `delq') to be
negligible, because it's a simple loop 100% implemented in C (in the
mapcar, most of the time is likely to be spent in the function-call made
for each element, and in the remove-if a similar amount of time is
probably spent in interpreting the byte-code that performs the
iteration).

>> Yes, cl-* functions are definitely allowed.  There is of course a lot of
>> carried prejudice from when cl-* functions didn't exist (and we just
>> had the `remove-if-not' instead, whose use was not accepted in Emacs's
>> own code), but there is also still a restriction in this respect: cl-*
>> functions still can't be used from preloaded files (because that would
>> require preloading cl-lib).
> By preloaded files, do you mean the ones on `preloaded-file-list'?

Yes.

> There are 112 files in this list on my system, so it's quite a large
> restriction.

Indeed.  It is very slightly relaxed by the fact that some of those cl-*
functions come with compiler-macros so some calls can be used because
they can be macro-expanded away during compilation (e.g. cl-list* and
cl-caddr, and I may have a patch somewhere that does it for cl-remove-if
and cl-remove-if-not).

> Is it no-more, no-less, i.e. only these 112 files?

That's right.

> Maybe the byte compiler could do some linting to enforce this
> restriction?  It already contains a lot of checks.

There is such a check already: if you use (eval-when-compile (require
'cl-lib)), the compiler should tell you if you use a function from
cl-lib.  And if you use (require 'cl-lib) instead, the compiler will
stay silent, but the "dump" phase (where we preload the files) should
complain that you're requiring a file that isn't explicitly loaded.


        Stefan



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-01-13 19:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-08 17:59 [PATCH] What's the quickest way to contribute? Oleh Krehel
2015-01-08 18:48 ` Eli Zaretskii
2015-01-08 19:04   ` Samer Masterson
2015-01-08 19:13     ` Dmitry Gutov
2015-01-09  0:01     ` Richard Stallman
2015-01-13  2:57 ` Dmitry Gutov
2015-01-13  5:10   ` Stefan Monnier
2015-01-13 11:15     ` Dmitry Gutov
2015-01-13 19:24       ` Stefan Monnier
2015-01-13 13:46     ` Oleh Krehel
2015-01-13 19:32       ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).