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/26 22:09:23 UTC
[GitHub] couchdb-fauxton pull request: docs: cleanup old views
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/338
docs: cleanup old views
remove a lot of old cold that we don't need any more after we
refactored the all-docs-list
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton cleanup-docs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/338.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 #338
----
commit 6ed373ac14324ed8b88d0721cd71c2fd1b59ab7e
Author: Robert Kowalski <ro...@apache.org>
Date: 2015-03-26T21:05:34Z
docs: cleanup old views
remove a lot of old cold that we don't need any more after we
refactored the all-docs-list
----
---
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-fauxton pull request: docs: cleanup old views
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27256636
--- Diff: app/addons/documents/shared-resources.js ---
@@ -27,7 +27,7 @@ define([
return FauxtonAPI.constants.DOC_URLS.GENERAL;
},
url: function (context) {
- if (context === undefined) {
+ if (!context) {
--- End diff --
Again, slight logic change here (if context is 0 or false this will execute) This okay?
I kinda like _.isUndefined(). Verbose but explicit!
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/338
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#issuecomment-87646511
rebased and merged as
79d5f320a4a708cf7c2bb5dad3480f0953e402de
5a10f1ca67347709d5654c4f5f775c63c8866e26
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27261179
--- Diff: app/addons/documents/shared-resources.js ---
@@ -27,7 +27,7 @@ define([
return FauxtonAPI.constants.DOC_URLS.GENERAL;
},
url: function (context) {
- if (context === undefined) {
+ if (!context) {
--- End diff --
i think the most cleanest way would be to remove the lazy context choosing, i grepped the code and just saw two occurences.
that way you have to pass the argument every time explicitly and the error logging in the url registry works in a reliable way
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27260831
--- Diff: app/addons/documents/shared-resources.js ---
@@ -27,7 +27,7 @@ define([
return FauxtonAPI.constants.DOC_URLS.GENERAL;
},
url: function (context) {
- if (context === undefined) {
+ if (!context) {
--- End diff --
some background info, sorry for the short answer:
the implementation is meant to take care of calls without a parameter: `.url()` - if that is the case it will choose to return the url for `server`.
this is also true for `.url(undefined)` - I am not sure if the user wants the url for the server if passing `undefined`
---
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-fauxton pull request: docs: cleanup old views
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27256540
--- Diff: app/addons/documents/shared-resources.js ---
@@ -36,9 +36,9 @@ define([
initialize: function (_attrs, options) {
if (this.collection && this.collection.database) {
this.database = this.collection.database;
- } else if (options.database) {
- this.database = options.database;
+ return;
}
+ this.database = options.database;
--- End diff --
Seems like the logic changed a little here. The old version only set this value if options.database evaluates to true; this always sets 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-fauxton pull request: docs: cleanup old views
Posted by sebastianrothbucher <gi...@git.apache.org>.
Github user sebastianrothbucher commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#issuecomment-87081266
Isn't most of the code going to disappear anyway as react is introduced further? (then it makes sense to just don't refactor the old code any more indeed). BTW: from a fucntional point of view the PR seems to work OK
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27257821
--- Diff: app/addons/documents/shared-resources.js ---
@@ -36,9 +36,9 @@ define([
initialize: function (_attrs, options) {
if (this.collection && this.collection.database) {
this.database = this.collection.database;
- } else if (options.database) {
- this.database = options.database;
+ return;
}
+ this.database = options.database;
--- End diff --
The model can't work in any way without setting a database (e.g. see `url` method)
---
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-fauxton pull request: docs: cleanup old views
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#issuecomment-87642306
+1 thanks.
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#issuecomment-87258767
we still use the backbone models for accessing the api endpoints
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#discussion_r27257380
--- Diff: app/addons/documents/shared-resources.js ---
@@ -27,7 +27,7 @@ define([
return FauxtonAPI.constants.DOC_URLS.GENERAL;
},
url: function (context) {
- if (context === undefined) {
+ if (!context) {
--- End diff --
who calls `Model.url(0);` and why? and if they do I guess they are happy with an url from the server
---
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-fauxton pull request: docs: cleanup old views
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/338#issuecomment-86879985
hi @benkeen. i removed the code in question, so we don't refactor the old code any more. please also read my comments above.
---
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.
---