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

remove sneakyThrow #2664

Closed
wants to merge 2 commits into from
Closed

Conversation

googl3r
Copy link

@googl3r googl3r commented May 23, 2021

It's my first ever contributions in an open source project, I use vavr a lot and a appreciate the author view in java.
if their is some errors I can update.

@googl3r googl3r requested a review from danieldietrich as a code owner May 23, 2021 15:32
@codecov-commenter
Copy link

Codecov Report

Merging #2664 (a966a45) into master (4fcb945) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2664      +/-   ##
============================================
- Coverage     92.89%   92.88%   -0.01%     
+ Complexity     5371     5370       -1     
============================================
  Files            89       89              
  Lines         12790    12790              
  Branches       1620     1620              
============================================
- Hits          11881    11880       -1     
  Misses          721      721              
- Partials        188      189       +1     
Impacted Files Coverage Δ Complexity Δ
src/main/java/io/vavr/control/Try.java 96.78% <100.00%> (+0.32%) 103.00 <0.00> (ø)
src/main/java/io/vavr/concurrent/FutureImpl.java 73.68% <0.00%> (-1.51%) 27.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4fcb945...a966a45. Read the comment docs.

@danieldietrich
Copy link
Contributor

Hi @googl3r, thanks a lot, I really appreciate it!

I don't think it is that simple, and I won't recommend to take this issue as a "first time committer" issue.
Sneaky throw was introduced with Vavr 0.9.0. Here is the PR that added it.
Especially, we removed the Fatal and NonFatal exception wrappers in Try.

There have been additional additions regarding sneaky throw, like this commit.

Please let me perform this change. It is more cross-cutting throughout the lib as it might seem. Also some methods need to explicitly declare which runtime exceptions are thrown, e.g. NoSuchElementException for Option.get() and NonFatalException for Try.get() etc.

Maybe we will find an issue for you that is better suited. 😅 From time to time I label issues with 'help wanted'. Look out for these.

Thank you!
Daniel

@googl3r
Copy link
Author

googl3r commented May 24, 2021

Hi @danieldietrich thank you for replying.

I will do as you said, search for simpler issue to start with. thanks a lot.

@danieldietrich
Copy link
Contributor

Thank you!

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