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

GORA-530: Removed redundant logging on several try/catch #130

Merged
merged 1 commit into from Apr 2, 2018

Conversation

alfonsonishikawa
Copy link
Member

Hi!
In this patch basically I removed the redundand "logging + throw" when the log message is the same as in the exception.
Sorry for the mess of two pull requests :(
Please comment anything you find that don't convice you.

Thanks!

https://issues.apache.org/jira/browse/GORA-530

@renato2099
Copy link
Contributor

Hi @alfonsonishikawa , thanks for doing this!
I think the first option was also ok because one thing is logging what the problem was and other throwing the exception for the application using Gora to deal with it as it fits better to the application.
what do you think @lewismc ?

@lewismc
Copy link
Member

lewismc commented Mar 24, 2018

@alfonsonishikawa @renato2099 I think this looks good. Tidies up Exception handling quite nicely.

@lewismc
Copy link
Member

lewismc commented Apr 2, 2018

@alfonsonishikawa can you commit to master?

@asfgit asfgit merged commit b782a0a into apache:master Apr 2, 2018
@alfonsonishikawa
Copy link
Member Author

Done, thank you!

@alfonsonishikawa
Copy link
Member Author

I can't believe how sausage-fingers I must be. I left a strange Eclipse autoimport at https://github.com/apache/gora/blob/master/gora-core/src/main/java/org/apache/gora/persistency/impl/BeanFactoryImpl.java#L28 . If you don't mind, I will straight delete it, test and push without a pull request -it is not being used, safe-. How embarrassing. Sorry.

@lewismc
Copy link
Member

lewismc commented Apr 3, 2018

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants