You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@accumulo.apache.org by dhutchis <gi...@git.apache.org> on 2016/10/08 15:51:26 UTC

[GitHub] accumulo pull request #162: ACCUMULO-4494 Add families and inclusive to Iter...

GitHub user dhutchis opened a pull request:

    https://github.com/apache/accumulo/pull/162

    ACCUMULO-4494 Add families and inclusive to IteratorTestInput

    Changed the relevant tests to use the new information.
    Added equals() and hashCode() for good measure.
    By default the families is empty and inclusive is false,
    so no existing code should break.
    Inserted a couple side changes to clear compiler warnings.

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

    $ git pull https://github.com/dhutchis/accumulo ACCUMULO-4494

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

    https://github.com/apache/accumulo/pull/162.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 #162
    
----
commit 397584803ffaa8c6170309c1fc8a015b23a8c3ad
Author: Dylan Hutchison <dh...@cs.washington.edu>
Date:   2016-10-08T15:39:22Z

    ACCUMULO-4494 Add families and inclusive to IteratorTestInput
    
    Changed the relevant tests to use the new information.
    Added equals() and hashCode() for good measure.
    By default the families is empty and inclusive is false,
    so no existing code should break.
    Inserted a couple side changes to clear compiler warnings.

----


---
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] accumulo issue #162: ACCUMULO-4494 Add families and inclusive to IteratorTes...

Posted by dhutchis <gi...@git.apache.org>.

Github user dhutchis commented on the issue:

    https://github.com/apache/accumulo/pull/162
  
    As an aside, it would be nice to make `AccumuloTestInput` a final class, which would guarantee its immutability.
    
    I would also like to make the helper methods in all the test case classes (such as `IsolatedDeepCopiesTestCase`) private.
    
    However, both these changes could break a user's code.


---
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] accumulo issue #162: ACCUMULO-4494 Add families and inclusive to IteratorTes...

Posted by joshelser <gi...@git.apache.org>.

Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/162
  
    >  By default the families is empty and inclusive is false,
    so no existing code should break.
    
    Agreed.
    
    > I would also like to make the helper methods in all the test case classes (such as IsolatedDeepCopiesTestCase) private.
    
    I think this is fine. Users running the framework shouldn't be looking inside the test cases. This is also outside of public API (which should be corrected...).
    
    Thanks for putting this together @dhutchis. Let me express my enthusiasm by committing for you :)


---
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] accumulo issue #162: ACCUMULO-4494 Add families and inclusive to IteratorTes...

Posted by joshelser <gi...@git.apache.org>.

Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/162
  
    Ok, running a test and will commit. I did make two minors changes:
    
    1. Removed the assert in favor of throwing an IllegalStateException (to match convention)
    2. Switched the null-check and `getClass()` check in favor of `instanceof` which does both.


---
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] accumulo issue #162: ACCUMULO-4494 Add families and inclusive to IteratorTes...

Posted by dhutchis <gi...@git.apache.org>.

Github user dhutchis commented on the issue:

    https://github.com/apache/accumulo/pull/162
  
    Thanks Josh!


---
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] accumulo pull request #162: ACCUMULO-4494 Add families and inclusive to Iter...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/accumulo/pull/162


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