Skip to content

[WIP] Implement BanData#138

Closed
jamierocks wants to merge 7 commits intoSpongePowered:feature/bansfrom
jamierocks:feature/bans
Closed

[WIP] Implement BanData#138
jamierocks wants to merge 7 commits intoSpongePowered:feature/bansfrom
jamierocks:feature/bans

Conversation

@jamierocks
Copy link
Contributor

As this is my first time implementing something from the Data API, I am making the process open so I don't make a mess of things.

  • SpongeBanData - complete
  • ImmutableSpongeBanData - complete
  • BanDataProccesor - should be complete
  • UserBansValueProccesor - should be complete
  • Register everything - correct me if i have missed something

Upon completion I will squash all the commits, unless you'd rather do that.

tldr; please correct me. pr is not yet finished.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't just a Set, it's a Set<Ban>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure thats what I did :S
Fixing now.

@jamierocks
Copy link
Contributor Author

This should be ready to merge, would you like me to squash the commits?

Copy link
Member

Choose a reason for hiding this comment

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

no comparison?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@gabizou gabizou self-assigned this Aug 18, 2015
@gabizou
Copy link
Member

gabizou commented Aug 19, 2015

Please update according to the changes recently merged.

@gabizou
Copy link
Member

gabizou commented Aug 24, 2015

@jamierocks Am I correct in thinking that you spoke with @Aaron1011 about re-representing bans and whitelist as a service perspective instead of a Data perspective in the API?

@jamierocks
Copy link
Contributor Author

Yes you are. As to why I haven't updated this PR.

@gabizou
Copy link
Member

gabizou commented Aug 24, 2015

@jamierocks So I would think that this PR should be closed if such an API change is going to be made (since I did speak to him about approving the change).

@jamierocks
Copy link
Contributor Author

Yeah, it should. (I was waiting for confirmation from you).

@jamierocks jamierocks closed this Aug 24, 2015
@jamierocks jamierocks deleted the feature/bans branch August 24, 2015 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants