Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEODE-7407: Remove AttributesFactory (4/4) #4301

Merged
merged 1 commit into from Nov 8, 2019

Conversation

jujoramos
Copy link
Contributor

This is one the commits to remove the deprecated 'AttributesFactory'
class from the geode-assembly module. The work is splitted to avoid
hitting issues in CI.

  • Fixed some minor warnings.
  • Replaced usages of 'junit.Assert' by 'assertj'.

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

This is one the commits to remove the deprecated 'AttributesFactory'
class from the geode-assembly module. The work is splitted to avoid
hitting issues in CI.

- Fixed some minor warnings.
- Replaced usages of 'junit.Assert' by 'assertj'.
@jujoramos jujoramos changed the title GEODE-7407: Remove AttributesFactory (4/4) GEODE-7407: Remove AttributesFactory (3/4) Nov 8, 2019
@jujoramos jujoramos changed the title GEODE-7407: Remove AttributesFactory (3/4) GEODE-7407: Remove AttributesFactory (4/4) Nov 8, 2019
InternalDistributedSystem ds = InternalDistributedSystem.getConnectedInstance();
invocationCount++;
ArrayList<String> l = (ArrayList<String>) context.getArguments();
if (l != null) {
assertFalse(Collections.disjoint(l, ds.getDistributedMember().getGroups()));
assertThat(Collections.disjoint(l, ds.getDistributedMember().getGroups())).isFalse();
Copy link

Choose a reason for hiding this comment

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

I am unable to see; the changes in this PR having any relation to attributes factory removal.
Anyways the change looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agingade : thanks for reviewing and approving this!.
Long story short: there was a single PR with 2 commits: one removing usages of AttributesFactory and the other one fixing warnings, removing usages of other deprecated classes and replacing junit.Assert by assert4j.
Due to issues while running StressNewTest with 4 test files changed, I had to split the single PR in 4 (one for each test class), that's the only reason why the name doesn't match, I'll make sure to put something more meaningful while doing the squash and merge.

@jujoramos jujoramos merged commit 0caabdb into apache:develop Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants