You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2015/03/01 15:44:48 UTC
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/28
add _changes?feed=stream sugar for continuous
allow `feed=stream` as sugar for `continuous` which is hard to
type.
closes COUCHDB-2237
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-chttpd 2237-stream
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/28.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #28
----
commit 53b35d9977c8a3947b6ca51d2826adf0892bfe5c
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-01T14:42:24Z
add _changes?feed=stream sugar for continuous
allow `feed=stream` as sugar for `continuous` which is hard to
type.
closes COUCHDB-2237
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/28
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/28
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
GitHub user robertkowalski reopened a pull request:
https://github.com/apache/couchdb-chttpd/pull/28
add _changes?feed=stream sugar for continuous
allow `feed=stream` as sugar for `continuous` which is hard to
type.
PRs for the change:
https://github.com/apache/couchdb/pull/307
https://github.com/apache/couchdb-couch/pull/40
https://github.com/apache/couchdb-chttpd/pull/28
closes COUCHDB-2237
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-chttpd 2237-stream
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/28.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #28
----
commit faa3d9d4d259a72cf6f8a31123e52b1d570e2c27
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-01T14:42:24Z
add _changes?feed=stream sugar for continuous
allow `feed=stream` as sugar for `continuous` which is hard to
type.
PRs for the change:
https://github.com/apache/couchdb/pull/307
https://github.com/apache/couchdb-couch/pull/40
https://github.com/apache/couchdb-chttpd/pull/28
closes COUCHDB-2237
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26791206
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
a bit of clarification, I most care about the error message being clear and I suggest we use @kxepal’s version, if we add the alias. I don’t advocate for or against the alias, but if others think, that it is a good idea, you should go ahead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26732892
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
`Supported `feed` types: normal, continuous, stream, longpoll, eventsource` e.g. without making difference between continuous and stream since both are the valid options. All the questions "why" will be covered by the docs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26725824
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
in general a person can misread every information you present and in this case i would doubt that this happens often
how would you present the message?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by janl <gi...@git.apache.org>.
Github user janl commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26746285
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
I’m with @rnewson.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-85257576
Voting 0 with no modifier as per same reason as @janl and @rnewson. Also agree using a comma instead of the slash when listing the possible values.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-87258639
Hum, I changed the error message, but as nobody is really a fan of it (so far -0 and 0) and I am also unsure about it I am going to close this and will give Dale feedback.
One thing left is that replication still uses continuous and stream would not be a good name for it. So I think we need to think about it more.
Maybe we can change `continuous` in version 3 as a breaking change for both (changes feed and replication).
Thanks for all your feedback!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-116790075
@robertkowalski ping (:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-87110106
thank you all for the input. fixed the error message. mind taking a second look @davisp?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26745607
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
I agree with @kxepal that `stream` should be listed like the other options.
I'm -0 on adding an alias for an existing feature (that is, I'd rather we didn't add more api without adding functionality, but I'm not blocking it).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-117313874
thanks for the reminder - merged as ca0195f38645b770c63715022ca14b7cdf649208
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26725879
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
@daleharvey what do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-117160127
feed=live already landed in couchdb-chttpd master, shouldn't this be closed now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: add _changes?feed=stream sugar for co...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#discussion_r26645505
--- Diff: src/chttpd_db.erl ---
@@ -93,7 +93,7 @@ handle_changes_req1(#httpd{}=Req, Db) ->
couch_stats:decrement_counter([couchdb, httpd, clients_requesting_changes])
end;
_ ->
- Msg = <<"Supported `feed` types: normal, continuous, longpoll, eventsource">>,
+ Msg = <<"Supported `feed` types: normal, continuous/stream, longpoll, eventsource">>,
--- End diff --
One thought just crossed my mind: don't you think, that `continuous/stream` could be misread by a user and they'll try to send a request with `?feed=continuous/stream` parameter?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---