Discussion:
Sorry -- patch review is going slowly (and how you can help)
Nick Mathewson
2014-06-09 03:04:07 UTC
Permalink
Hi, all!

Things have been busy in my day job, so please accept my apologies for
the slow speed of getting incoming patches reviewed. I'll try to set
aside time in June to reduce those queues as much as I can.

You can help! If you've looked around the Libevent codebase much at
all, you could have a look through patches and bug reports at the
various trackers that people seem to be using.

There's the one I'd prefer people to use for patches:

https://github.com/libevent/libevent/

And there's the other two:

https://github.com/nmathewson/libevent/
http://sourceforge.net/projects/levent/

When reviewing patches, here are some things to look for:
* If this is a patch to libevent itself, is it tested? Is it
correct? (Whenever possible, changes should have tests.)
* If this is a patch to the documentation or the sample code, does
it really improve clarity?
* If this patch introduces any new APIs, are they well-documented,
reasonable, and relatively future-proot?
* If this patch changes any existing APIs, will the change break any
existing correct code? (If so, the patch must change. We don't do
this kind of API breakage.)


cheers,
--
Nick
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Azat Khuzhin
2014-09-18 14:50:24 UTC
Permalink
Post by Nick Mathewson
Hi, all!
Things have been busy in my day job, so please accept my apologies for
the slow speed of getting incoming patches reviewed. I'll try to set
aside time in June to reduce those queues as much as I can.
You can help! If you've looked around the Libevent codebase much at
all, you could have a look through patches and bug reports at the
various trackers that people seem to be using.
https://github.com/libevent/libevent/
https://github.com/nmathewson/libevent/
http://sourceforge.net/projects/levent/
* If this is a patch to libevent itself, is it tested? Is it
correct? (Whenever possible, changes should have tests.)
* If this is a patch to the documentation or the sample code, does
it really improve clarity?
* If this patch introduces any new APIs, are they well-documented,
reasonable, and relatively future-proot?
* If this patch changes any existing APIs, will the change break any
existing correct code? (If so, the patch must change. We don't do
this kind of API breakage.)
Hi Nick,

Could you please look through this simple patches:
https://github.com/libevent/libevent/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee+author%3Aazat+created%3A2014-03-01..2014-08-01+

Cheers,
Azat.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Azat Khuzhin
2014-09-18 15:07:47 UTC
Permalink
Post by Azat Khuzhin
Post by Nick Mathewson
Hi, all!
Things have been busy in my day job, so please accept my apologies for
the slow speed of getting incoming patches reviewed. I'll try to set
aside time in June to reduce those queues as much as I can.
You can help! If you've looked around the Libevent codebase much at
all, you could have a look through patches and bug reports at the
various trackers that people seem to be using.
https://github.com/libevent/libevent/
https://github.com/nmathewson/libevent/
http://sourceforge.net/projects/levent/
* If this is a patch to libevent itself, is it tested? Is it
correct? (Whenever possible, changes should have tests.)
* If this is a patch to the documentation or the sample code, does
it really improve clarity?
* If this patch introduces any new APIs, are they well-documented,
reasonable, and relatively future-proot?
* If this patch changes any existing APIs, will the change break any
existing correct code? (If so, the patch must change. We don't do
this kind of API breakage.)
Hi Nick,
https://github.com/libevent/libevent/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee+author%3Aazat+created%3A2014-03-01..2014-08-01+
Also this is very simple, and correct from my point of view:
https://github.com/libevent/libevent/pull/123
(Otherwise we will comparing only first one byte from evb_two buffer)
Post by Azat Khuzhin
Cheers,
Azat.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Nick Mathewson
2014-09-18 15:48:19 UTC
Permalink
[...]
Post by Azat Khuzhin
Post by Azat Khuzhin
Hi Nick,
https://github.com/libevent/libevent/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee+author%3Aazat+created%3A2014-03-01..2014-08-01+
https://github.com/libevent/libevent/pull/123
(Otherwise we will comparing only first one byte from evb_two buffer)
Thanks for the reminder! I've just had a look over these, merged a
few, and commented on the rest.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Azat Khuzhin
2014-09-29 22:10:05 UTC
Permalink
Post by Nick Mathewson
[...]
Post by Azat Khuzhin
Post by Azat Khuzhin
Hi Nick,
https://github.com/libevent/libevent/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee+author%3Aazat+created%3A2014-03-01..2014-08-01+
https://github.com/libevent/libevent/pull/123
(Otherwise we will comparing only first one byte from evb_two buffer)
Thanks for the reminder! I've just had a look over these, merged a
few, and commented on the rest.
Hi Nick,

Could you please look at this patches:
- https://github.com/libevent/libevent/pull/170
https-client: add -retries argument, for connection retries
(This one is used by #171)

- https://github.com/libevent/libevent/pull/171
bufferevent_openssl: reset fd_is_set when setfd with -1 is called

- https://github.com/libevent/libevent/pull/174
Dns fail disable when inactive fix v3

We already have discussion for this one, I responded to you here
https://github.com/libevent/libevent/pull/131#commitcomment-7838163
But I found one more problem there (I was confused because of too many
topic branches not merged yet), it triggered by
dns/retry_disable_when_inactive, and the last patch fixes the issue.

- *http retry*
I also have patch for http retry code (there are some issues with it),
but I want cover it by regression tests before submitting it upstream.

Cheers,
Azat.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.
Azat Khuzhin
2014-09-30 14:54:47 UTC
Permalink
Post by Azat Khuzhin
Post by Nick Mathewson
[...]
Post by Azat Khuzhin
Post by Azat Khuzhin
Hi Nick,
https://github.com/libevent/libevent/pulls?q=is%3Aopen+is%3Apr+no%3Aassignee+author%3Aazat+created%3A2014-03-01..2014-08-01+
https://github.com/libevent/libevent/pull/123
(Otherwise we will comparing only first one byte from evb_two buffer)
Thanks for the reminder! I've just had a look over these, merged a
few, and commented on the rest.
Hi Nick,
- https://github.com/libevent/libevent/pull/170
https-client: add -retries argument, for connection retries
(This one is used by #171)
- https://github.com/libevent/libevent/pull/171
bufferevent_openssl: reset fd_is_set when setfd with -1 is called
- https://github.com/libevent/libevent/pull/174
Dns fail disable when inactive fix v3
We already have discussion for this one, I responded to you here
https://github.com/libevent/libevent/pull/131#commitcomment-7838163
But I found one more problem there (I was confused because of too many
topic branches not merged yet), it triggered by
dns/retry_disable_when_inactive, and the last patch fixes the issue.
- *http retry*
I also have patch for http retry code (there are some issues with it),
but I want cover it by regression tests before submitting it upstream.
Here is a pull request for this one:
https://github.com/libevent/libevent/pull/175

But there are some problems with regress for this patch.

Cheers,
Azat.
***********************************************************************
To unsubscribe, send an e-mail to ***@freehaven.net with
unsubscribe libevent-users in the body.

Continue reading on narkive:
Loading...