Skip to content

Conversation

@ctooley21
Copy link
Contributor

Changes:
Fixed NPE
Had to put a UTF-8 line in gradle as I was getting odd errors.
Vault API updates.

@ctooley21 ctooley21 mentioned this pull request Jun 10, 2017
Copy link
Member

@markhughes markhughes left a comment

Choose a reason for hiding this comment

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

Everything is perfect, I will merge this now.

I added some notes about styling changes from the previous maintainers, which we can change in another commit 👍

Thanks for working on this so quickly 🕺


// Transfer money
EconomyResponse erw = econ.withdrawPlayer(fromAcc, amount);
EconomyResponse erw = econ.withdrawPlayer(fromAcc.getName(), amount);
Copy link
Member

Choose a reason for hiding this comment

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

I know its not your fault, but the previous maintainer used a lot of acronyms which we're trying to move away from - as these can make it difficult to read.

For example, erw should be ecomomyResponseWithdraw


if (erw.transactionSuccess()) {
EconomyResponse erd = econ.depositPlayer(toAcc, amount);
EconomyResponse erd = econ.depositPlayer(toAcc.getName(), amount);
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, move to ecomonyResponseDeposit


public TextUtil() {
this.tags = new HashMap<String, String>();
this.tags = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Perfect!! Diamonds all around 💎💎💎💎

@markhughes markhughes merged commit 7ba9e42 into redstone:master Jun 11, 2017
@markhughes markhughes mentioned this pull request Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants