You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by felizbear <gi...@git.apache.org> on 2016/12/29 08:16:54 UTC
[GitHub] zeppelin pull request #1817: remove usage of non-standard string method
GitHub user felizbear opened a pull request:
https://github.com/apache/zeppelin/pull/1817
remove usage of non-standard string method
### What is this PR for?
This PR removes a non-standard string prototype method `contains` that can cause potential bugs in the future maintenance.
### What type of PR is it?
Bug Fix
### Todos
* [x] - remove usage of non-standard string method `contains` in favor of standard `indexOf`
### How should this be tested?
Download as `csv / tsv` (graph view) should work as expected
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/felizbear/zeppelin remove-prototype-method
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1817.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 #1817
----
commit d8e9005ccdfcac296be89f5227966589f11a6fa2
Author: felizbear <il...@nflabs.com>
Date: 2016-12-29T08:08:52Z
remove usage of non-standard string method
use standard `indexOf` method instead of proprietary `contains` defined
on string by unknown library; it is a potential bug; also it doesn't add
much value at all, compare:
stringValue.contains(delimiter)
stringValue.indexOf(delimiter) > -1
----
---
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] zeppelin pull request #1817: remove usage of non-standard string method
Posted by felizbear <gi...@git.apache.org>.
Github user felizbear closed the pull request at:
https://github.com/apache/zeppelin/pull/1817
---
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] zeppelin issue #1817: remove usage of non-standard string method
Posted by felizbear <gi...@git.apache.org>.
Github user felizbear commented on the issue:
https://github.com/apache/zeppelin/pull/1817
I've retriggered CI a few times. Current error is
```
Failed tests:
NotebookTest.testAbortParagraphStatusOnInterpreterRestart:760 expected:<ABORT> but was:<RUNNING>
```
obviously not related to the change. Can we ignore it and merge anyway?
---
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] zeppelin pull request #1817: remove usage of non-standard string method
Posted by felizbear <gi...@git.apache.org>.
GitHub user felizbear reopened a pull request:
https://github.com/apache/zeppelin/pull/1817
remove usage of non-standard string method
### What is this PR for?
Recreating #1701 that was lost in refactoring.
This PR removes a non-standard string prototype method `contains` that can cause potential bugs in the future maintenance.
### What type of PR is it?
Bug Fix
### Todos
* [x] - remove usage of non-standard string method `contains` in favor of standard `indexOf`
### How should this be tested?
Download as `csv / tsv` (graph view) should work as expected
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/felizbear/zeppelin remove-prototype-method
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1817.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 #1817
----
commit d8e9005ccdfcac296be89f5227966589f11a6fa2
Author: felizbear <il...@nflabs.com>
Date: 2016-12-29T08:08:52Z
remove usage of non-standard string method
use standard `indexOf` method instead of proprietary `contains` defined
on string by unknown library; it is a potential bug; also it doesn't add
much value at all, compare:
stringValue.contains(delimiter)
stringValue.indexOf(delimiter) > -1
----
---
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] zeppelin issue #1817: remove usage of non-standard string method
Posted by soralee <gi...@git.apache.org>.
Github user soralee commented on the issue:
https://github.com/apache/zeppelin/pull/1817
Tested it and works as expected.
But CI is fail. Could you check CI again?
---
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] zeppelin pull request #1817: remove usage of non-standard string method
Posted by felizbear <gi...@git.apache.org>.
Github user felizbear closed the pull request at:
https://github.com/apache/zeppelin/pull/1817
---
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] zeppelin pull request #1817: remove usage of non-standard string method
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1817
---
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] zeppelin pull request #1817: remove usage of non-standard string method
Posted by felizbear <gi...@git.apache.org>.
GitHub user felizbear reopened a pull request:
https://github.com/apache/zeppelin/pull/1817
remove usage of non-standard string method
### What is this PR for?
Recreating #1701 that was lost in refactoring.
This PR removes a non-standard string prototype method `contains` that can cause potential bugs in the future maintenance.
### What type of PR is it?
Bug Fix
### Todos
* [x] - remove usage of non-standard string method `contains` in favor of standard `indexOf`
### How should this be tested?
Download as `csv / tsv` (graph view) should work as expected
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/felizbear/zeppelin remove-prototype-method
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1817.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 #1817
----
commit d8e9005ccdfcac296be89f5227966589f11a6fa2
Author: felizbear <il...@nflabs.com>
Date: 2016-12-29T08:08:52Z
remove usage of non-standard string method
use standard `indexOf` method instead of proprietary `contains` defined
on string by unknown library; it is a potential bug; also it doesn't add
much value at all, compare:
stringValue.contains(delimiter)
stringValue.indexOf(delimiter) > -1
----
---
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] zeppelin issue #1817: remove usage of non-standard string method
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1817
LGTM and merge to master if there're no further discussions.
---
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.
---