Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-944

Deprecate Graph.Exceptions.elementNotFound

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Done
    • Affects Version/s: 3.0.2-incubating
    • Fix Version/s: 3.2.3
    • Component/s: structure
    • Labels:
      None

      Description

      As it sits right now, the standard elementNotFound exception is not used in the core code base, but we are validating that it is being thrown in the test suite by providers! The test is actually faulty because it is not asserting the message and just that it is a NoSuchElementException which is causing the problem. We should either deprecate this method and just validate NoSuchElementException or use it in full and force providers to do the same.

      This has the potential to be a breaking change as it might break tests if providers aren't using that standard exception.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tinkerpop/pull/436

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/436
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user okram commented on the issue:

          https://github.com/apache/tinkerpop/pull/436

          VOTE +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/436 VOTE +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dkuppitz commented on the issue:

          https://github.com/apache/tinkerpop/pull/436

          VOTE: +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/436 VOTE: +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

          https://github.com/apache/tinkerpop/pull/436

          TINKERPOP-944 Deprecate Graph.Exceptions.elementNotFound()

          https://issues.apache.org/jira/browse/TINKERPOP-944

          This exception wasn't used in the code base at all except for tests. Seems best to just deprecate (in case a graph provider is publically using it). Didn't change the nature of the tests that were using this method, so this should be a non-breaking change.

          The change is pretty simple, but I decided not to CTR since "deprecation" was involved. All tests good with `mvn clean install`.

          VOTE +1

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/apache/tinkerpop TINKERPOP-944

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/tinkerpop/pull/436.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 #436


          commit 83a4cf109d840a929b5cabcf1db584cc75378bc1
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2016-09-25T00:23:59Z

          Deprecate Graph.Exceptions.elementNotFound()

          This exception wasn't used in the code base at all except for tests. Seems best to just deprecate (in case a graph provider is publically using it). Didn't change the nature of the tests that were using this method, so this should be a non-breaking change.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/436 TINKERPOP-944 Deprecate Graph.Exceptions.elementNotFound() https://issues.apache.org/jira/browse/TINKERPOP-944 This exception wasn't used in the code base at all except for tests. Seems best to just deprecate (in case a graph provider is publically using it). Didn't change the nature of the tests that were using this method, so this should be a non-breaking change. The change is pretty simple, but I decided not to CTR since "deprecation" was involved. All tests good with `mvn clean install`. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-944 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/436.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 #436 commit 83a4cf109d840a929b5cabcf1db584cc75378bc1 Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-09-25T00:23:59Z Deprecate Graph.Exceptions.elementNotFound() This exception wasn't used in the code base at all except for tests. Seems best to just deprecate (in case a graph provider is publically using it). Didn't change the nature of the tests that were using this method, so this should be a non-breaking change.
          Hide
          spmallette stephen mallette added a comment -

          Removed the breaking label - this is not a breaking change as it is just a deprecation of two methods.

          Show
          spmallette stephen mallette added a comment - Removed the breaking label - this is not a breaking change as it is just a deprecation of two methods.
          Hide
          spmallette stephen mallette added a comment -

          Oops - added this to 3.1.0-incubating by mistake. force of habit i guess.

          Show
          spmallette stephen mallette added a comment - Oops - added this to 3.1.0-incubating by mistake. force of habit i guess.

            People

            • Assignee:
              spmallette stephen mallette
              Reporter:
              spmallette stephen mallette
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development