You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/08/05 20:45:14 UTC
[GitHub] [fineract] thesmallstar opened a new pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
thesmallstar opened a new pull request #1238:
URL: https://github.com/apache/fineract/pull/1238
Pleas check the discussion on: https://github.com/apache/fineract/pull/1006
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1238:
URL: https://github.com/apache/fineract/pull/1238#issuecomment-676506453
Yes I agree, that makes perfect sense; Thank You for the detailed explanation about this check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1238:
URL: https://github.com/apache/fineract/pull/1238#issuecomment-673856171
@vorburger would you be OK if we merge this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] thesmallstar commented on pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
thesmallstar commented on pull request #1238:
URL: https://github.com/apache/fineract/pull/1238#issuecomment-670168858
Take this code for example:
https://github.com/apache/fineract/blob/2323450e71d1508e6ff218d48b856aefa04542d0/fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/JournalEntryRunningBalanceUpdateServiceImpl.java#L243-L269
here, it wants us to add a default statement and from what I have understood from https://github.com/apache/fineract/pull/1006#issuecomment-641687223 by @ptuomola that
1. If we are adding new value to the enum, error-prone directly asks us to cover that in the switch case.
2. If there is no common way to handle things there is no need for a default statement.
In other words, in all of these cases all the values that are covered by the enum are always included thanks to error-prone, and also we don't have any common way of handling things, that means we would end up adding an incorrect/blank default statement that brings no benefit.
would you agree on this? @vorburger @ptuomola @percyashu ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1238:
URL: https://github.com/apache/fineract/pull/1238#issuecomment-672777409
@thesmallstar that makes sense to me. Either we have a default case to cover all the cases not covered, or we require all cases to be covered explicitly. As far as I can see, they are mutually exclusive...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola merged pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
ptuomola merged pull request #1238:
URL: https://github.com/apache/fineract/pull/1238
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on pull request #1238: Removed MissingSwitchDefault Checkstyle from TODO
Posted by GitBox <gi...@apache.org>.
ptuomola commented on pull request #1238:
URL: https://github.com/apache/fineract/pull/1238#issuecomment-675335152
No objection from @vorburger so far - I will merge this so that it makes the 1.4.0 release.
If there's any concern from anyone, we can easily add this back to the todos.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org