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

Temporary set of catch to ignored if they are ignored #222

Merged
merged 1 commit into from
Oct 2, 2014
Merged

Temporary set of catch to ignored if they are ignored #222

merged 1 commit into from
Oct 2, 2014

Conversation

thatsIch
Copy link
Member

@thatsIch thatsIch commented Oct 1, 2014

VERY temporary, need to rework the whole try catch logic. Needs major rework to be honest.

@PlasmaPower
Copy link

This does nothing other than make the source code nicer right? Am I missing something?

@thatsIch
Copy link
Member Author

thatsIch commented Oct 1, 2014

It tells the reader that the catch phrase is intentionally left out for now as ignored should be pretty self explanatory in comparison to e, o and t

@PlasmaPower
Copy link

@thatsIch OK. Usually it is pretty obvious if there is nothing in the catch, but it is still helpful for people who are new to Java.

@FireBall1725
Copy link
Contributor

that is a lot of catch blocks that do nothing

@thatsIch
Copy link
Member Author

thatsIch commented Oct 1, 2014

There might be cases where its wanted to be ignored and sometimes when its forgotten to do something there (for example at least logging it)

@thatsIch thatsIch added this to the rv2 milestone Oct 1, 2014
@thatsIch thatsIch added the enhancement An enhancement for an existing feature label Oct 1, 2014
@yueh
Copy link
Member

yueh commented Oct 1, 2014

Looks ok.

I have briefly discussed the point about catching throwables with Algo. By default it is a bad practice.
But he mentioned something about other mods not written properly and causing errors about class not found and similar things. But these still be still exceptions. So maybe we should reconsidering it and make the catches a bit more precise (and adding some logging to them).

Cisien added a commit that referenced this pull request Oct 2, 2014
Temporary set of catch to ignored if they are ignored
@Cisien Cisien merged commit 7ef413a into AppliedEnergistics:rv2 Oct 2, 2014
@thatsIch thatsIch deleted the ignored branch October 2, 2014 07:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An enhancement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants