Introduction to Method Handles tutorial#90
Conversation
danthe1st
left a comment
There was a problem hiding this comment.
Here are some opinions on that article. I am an outside contributor and I don't have any authority over this repository.
I noticed quite a few lines ending with a single space. Is this deliberate?
When introducing methods (e.g. in the Lookup, MethodHandle or MethodHandles classes), it might be a good idea to link Javadocs.
ammbra
left a comment
There was a problem hiding this comment.
Hey Nataliia,
I briefly left a few comments, but would also like to add the following general ones:
- Please add links to JavaDoc for respective classes or methods.
- Could you please add a bio similar to the one from JFall? 😄
|
Hi @danthe1st and @ammbra, thank you both for the reviews. |
|
Hi @ammbra and @danthe1st, I have pushed the changes with your comments addressed, could you please check? Along with your suggestions I have also reordered some sections: the |
danthe1st
left a comment
There was a problem hiding this comment.
Thank you for your changes. I have commented on minor things I noticed while reading through your changes.
|
Thanks @danthe1st! I have pushed the changes. |
danthe1st
left a comment
There was a problem hiding this comment.
Thank you for this article and the changes you have made. For what I am concerned, this article looks good.
ammbra
left a comment
There was a problem hiding this comment.
Hi Nataliia,
Thank you so much for updating the article. I left some small comments/suggestions, but overall is great work.
|
We're close!! After these last comments are resolved I think we'll look to get this merged! |
|
Hi @ammbra, thank you for the review and your help with fixes. I have made the changes, please let me know if it looks good or more adjustment required. |
danthe1st
left a comment
There was a problem hiding this comment.
I read through the article again and wrote down a few things I noticed.
|
Hi @smthelusive if you give me access to push commits I can go in and clean a few things up and help get this merged. I think we've gone back and forth long enough and I thank you for your patience and commitment to getting this over the line! |
|
Thank you for your help @carimura, I have sent you an invite. I'm still willing to fix whatever is missing, so feel free to leave anything on me, I will get to it as soon as I can. |
|
OK I added a few grammar edits and reduced a few lines of code that felt repeated in the same section. Good from my end! I see @ammbra left a few comments that I think are still unresolved (I agree the conclusion could be a bit stronger) but other than that, we should be good to go! If you @smthelusive are busy this week (what remains of it :) ) we might be able to assist Friday and get things merged. |
…methodhandle.md Co-authored-by: dan1st <daniel@wwwmaster.at>
…methodhandle.md Co-authored-by: dan1st <daniel@wwwmaster.at>
…methodhandle.md Co-authored-by: dan1st <daniel@wwwmaster.at>
…methodhandle.md Co-authored-by: dan1st <daniel@wwwmaster.at>
…methodhandle.md Co-authored-by: dan1st <daniel@wwwmaster.at>
ba66c0d to
14f7899
Compare
Method Handles article ready for review.