Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

add _changes?feed=stream sugar for continuous #28

Closed
wants to merge 1 commit into from

Conversation

robertkowalski
Copy link
Contributor

allow feed=stream as sugar for continuous which is hard to
type.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
#28

closes COUCHDB-2237

robertkowalski added a commit to robertkowalski/couchdb-couch that referenced this pull request Mar 1, 2015
allow `feed=stream` as sugar for `continuous` which is hard to
type.

PRs for the change:
apache/couchdb#307
apache#40
apache/couchdb-chttpd#28

closes COUCHDB-2237
robertkowalski added a commit to robertkowalski/couchdb-chttpd that referenced this pull request Mar 1, 2015
allow `feed=stream` as sugar for `continuous` which is hard to
type.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
apache#28

closes COUCHDB-2237
robertkowalski added a commit to robertkowalski/couchdb that referenced this pull request Mar 1, 2015
@@ -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">>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daleharvey what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m with @rnewson.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@davisp
Copy link
Member

davisp commented Mar 24, 2015

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.

robertkowalski added a commit to robertkowalski/couchdb-chttpd that referenced this pull request Mar 27, 2015
allow `feed=stream` as sugar for `continuous` which is hard to
type.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
apache#28

closes COUCHDB-2237
@robertkowalski
Copy link
Contributor Author

thank you all for the input. fixed the error message. mind taking a second look @davisp?

@robertkowalski
Copy link
Contributor Author

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!

robertkowalski added a commit to robertkowalski/couchdb that referenced this pull request Apr 4, 2015
@robertkowalski robertkowalski reopened this Apr 4, 2015
robertkowalski added a commit to robertkowalski/couchdb-chttpd that referenced this pull request Apr 4, 2015
allow `feed=live` as sugar for `continuous` which is hard to type.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
apache#28

closes COUCHDB-2237
robertkowalski added a commit to robertkowalski/couchdb-couch that referenced this pull request Apr 4, 2015
allow `feed=live` as sugar for `continuous` which is hard to
type. PouchDB already supports `live`.

PRs for the change:
apache/couchdb#307
apache#40
apache/couchdb-chttpd#28

closes COUCHDB-2237
allow `feed=live` as sugar for `continuous` which is hard to type.
PouchDB already supports `live`.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
apache#28

closes COUCHDB-2237
asfgit pushed a commit to apache/couchdb that referenced this pull request May 24, 2015
asfgit pushed a commit that referenced this pull request May 24, 2015
allow `feed=live` as sugar for `continuous` which is hard to type.
PouchDB already supports `live`.

PRs for the change:
apache/couchdb#307
apache/couchdb-couch#40
#28

closes COUCHDB-2237
asfgit pushed a commit to apache/couchdb-couch that referenced this pull request May 24, 2015
allow `feed=live` as sugar for `continuous` which is hard to
type. PouchDB already supports `live`.

PRs for the change:
apache/couchdb#307
#40
apache/couchdb-chttpd#28

closes COUCHDB-2237
@kxepal
Copy link
Member

kxepal commented Jun 29, 2015

@robertkowalski ping (:

@rnewson
Copy link
Member

rnewson commented Jun 30, 2015

feed=live already landed in couchdb-chttpd master, shouldn't this be closed now?

@robertkowalski
Copy link
Contributor Author

thanks for the reminder - merged as ca0195f

@robertkowalski robertkowalski deleted the 2237-stream branch June 30, 2015 19:29
andywenk pushed a commit to andywenk/couchdb that referenced this pull request Aug 31, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants