(Partially) Closes #351: Javadocs for map APIs

[jdimeo]

I have completed Javadocs on the API's map package. I am opening a PR to being the review process now before I (or someone else!) moves on to the other packages (list, set, bag, etc.). In my view, map was the highest priority with methods like getIfAbsentPutWith that benefited most from Javadocs. Additionally, I'm envisioning many Javadocs in those packages will be copy-paste from what I have here, so I want to confirm the verbiage is right before doing so.

Some non-Javadoc changes included in this PR (which can be taken out if needed):

• Update checkstyle DTD (now content assist works in Eclipse when editing the configs)
• Remove \r checkstyle check. I don't see what benefit it provides (from what I can tell, my commits still have just \n even from Windows) while just making it a lot harder to contribute from Windows and get a clean build
• Remove a redundant tap() declaration in immutableObjectPrimitiveMap.stg

Additionally, this includes a helper JavadocUtil that will copy Javadocs back from a generated class to a template. It's not perfect but it saved me a ton of time, so that I could edit the Javadocs on an example concrete class and benefit from Eclipse's Javadoc assists that would otherwise be unavailable when directly editing the template.

[donraab]

This sentence fragment looks like it might be superfluous.

[jdimeo]

Good catch! I'll fix it.

[donraab]

I don't believe the key is passed to the function, as it is a Function0 which means it takes no parameters.

[jdimeo]

That is what I meant, but I agree it's a bit confusing. By saying "value supplier" I was hoping that readers would realize a Function0 == Supplier<> from Java 8. I was trying to say "associates ... with the key" not "associates ... invoking ... with the key". I'll try to word it more clearly.

[donraab]

Hi @jdimeo, I've reviewed all of the docs. They look great for the most part, but I've request a few changes. This was surprisingly fun to read and I very much appreciate the time you have taken to write all this. This is a tremendous contribution to the framework. Thank you very much!

After you make the requested changes and rebase and squash your commits I will approve and merge.

One final thing, I saw the JavadocUtil class. This looks very interesting and useful. I think it would be great if you would be open to writing a blog or adding a wiki page with examples showing how to use this in practice.

[jdimeo]

@donraab thank you for the review, feedback, and encouragement! I saw this earlier in the week and wanted my next comment to be another push with the requested changes, but I was unable to get to this this week. It's on the to-do list for next week, and I'll see if I can knock out a couple other packages as well (set, bag, etc.)

[nikhilnanivadekar]

I would suggest let’s get this PR merged before you add more packages. That way the changes and commits are small and reviews and feedback can be faster

[nikhilnanivadekar]

@jdimeo I would like to close out the feature set for 10.3.0 and release it soon. That way we can open the major version 11.0.0 target. If possible, let us try to get this PR in as Javadocs help us throughout the development experience. Once all the comments suggested by @donraab are addressed, I will need your (@jdimeo ) help in a bit of house keeping especially to squash the merge commits and commits which have just stylistic changes. We can visit them once all the changes are approved. We are super close, and it would be great to have this merged. Thanks a ton for this valuable contribution!

[jdimeo]

@donraab @nikhilnanivadekar I've pushed a commit that addresses the review items. Don't you all squash when you merge the PR? I've reverted the tap() change so the only non-Javadoc thing remaining are the few minor Checkstyle things. Was that what you meant as far as needing me to do housekeeping?

Happy to contribute to a wiki or blog. Just let me know where/how to submit a draft for review (I see that it appears I can directly edit the wiki, but I assume you would want to review it first?)

[nikhilnanivadekar]

@jdimeo we normally like to squash commits, however, the changes in terms of LOC are pretty high, and I would like you to get the proper number of commit credit where it is due. So, let me elaborate the housekeeping things which I think we need to do before having a final pass review:

1. Fix-up 9d46f11 on 712cc71

2. Rearrange and fix-up 1719ab2 on ee6ea03

3. Fix-up 1b8fcec on e0bdfc4

4. Fix-up c3682f0 on e0bdfc4

5. Fix-up 8637734 on e0bdfc4

6. Fix-up 23d1ffe on b08407a

7. I am fine with this commit: 9cea83f
   I tried to fix it without removing, but it resulted in no luck. I would just re-arrange this commit to be close to the checkstyle commits which you made at the start of the PR. It just helps in writing Release Notes.

8. Fix up 431b0be on 712d5cd I don't want to be pedantic and make you split your change and fix-up the review comment changes on that particular commit hash. Instead I am fine if you fix-up the review comment changes commit to the last commit.

9. I did not see the commit which reverts the changes made for tap() but I might have missed something. So, if the tap() change is not reverted please revert it and fix-up on that particular commit where tap() was changed, so that the net effect looks to be zero. If you have already done this, sorry for missing that part.

10. Rebase on to upstream/master

After this we will do a final review, merge your PR and then I will cut a milestone release for testing.
Thanks again for your contribution!

[nikhilnanivadekar]

Elaborated house-keeping changes in the comments below.

[jdimeo]

@nikhilnanivadekar I've actually never done a "fix up" commit before since I usually work on smaller projects. But I understand now how that works with autosquash. I personally would've been fine with a massive "Javadoc all the things" commit, but this is definitely better. I'll address it tomorrow (I hope!)

P.S. I did revert the tap change so the net effect is zero (I snuck in the commit between donraab's first review comment and all the others so it's easy to miss) . Are you OK with me adding the @Override tag? The only reason why I noticed that method in the first place was because I got warnings in Eclipse about it overridding but not being tagged with the annotation.

[nikhilnanivadekar]

@jdimeo found the hidden commit for revert. So, can you fix-up e36ca66 on 391e4fc

[nikhilnanivadekar]

@jdimeo If you have squashed commits you have essentially done a fix-up. Fix up is nothing but squash where the commit message from previous commit is picked. For additional documentation take a look here:

1. https://git-scm.com/docs/git-rebase
2. https://stackoverflow.com/a/16758662
3. https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
4. https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

If you do an interactive rebase (points 3, 4 above), git gives you the chance to move around commits. Rebase can be dangerous, especially when doing it for the first time as it is essentially rewriting history. So I would suggest to take a back up of the files before doing the rebase.
Feel free to reach out with questions or concerns and I will be happy to help you through.

[nikhilnanivadekar]

You can add the Override tag, but please add it as a separate commit for tracking. It just helps me while I write Release Notes.

[jdimeo]

@nikhilnanivadekar doing the fixup of 23d1ffe created a weird merge conflict in the middle of the interactive rebase that I wasn't sure how to correctly resolve, Do I need to figure that out or can we roll with every other change except that one?

[nikhilnanivadekar]

Let’s roll with the others and then revisit this. I can help you offline with that one later

[jdimeo]

Thanks @nikhilnanivadekar I just forced push my history rewrite. Please tell me if I did it correctly! I also rebased on upstream/master so it should be ready to go

[nikhilnanivadekar]

@jdimeo actually now try again doing the fix up of 0798268 on 1e780c4
That should most likely go through. Give it a go once, please?

[nikhilnanivadekar]

Also a minor nit pick, can you reword this commit: a9f5d4b
to call it Javadocs to make it consistent with other commits. We are big fans of symmetry as you would see 😄 thanks again for your patience and cooperation.

[jdimeo]

@nikhilnanivadekar I allocated a short to track your patience quota with me but it just overflowed to an int- I don't know how you do it! thank you! :-P

[jdimeo]

@nikhilnanivadekar Yes! it worked this time no merge required. I also caught a missing " for " and added some . so they are all completely symmetrical now :-)

[nikhilnanivadekar]

@jdimeo I missed a couple more things, sorry!

1. Reword b0aa722 to keep only the change of Javadoc
2. Reword 20de3bf to make it succinct to convey only the removal for the check

[jdimeo]

@nikhilnanivadekar done (I hope!)

[jdimeo]

@nikhilnanivadekar Just confirming you don't need anything else from me. Also, I noticed this in .gitattributes:

# Auto detect text files and perform LF normalization
* text=auto

that could explain why we can safely remove the \r checkstyle check

[nikhilnanivadekar]

We might want to update the wording a bit in a future iteration. But it is not super important.
keysView() keyValueView() essentially all *View() APIs are supposed to be lazy. So, the idea of changes in the map reflecting in the view might be a bit awkward.
It is ok for now

[nikhilnanivadekar]

This utility is awesome! May be if it is possible, can you contribute another utility which will move code to the template? It will be super useful to be able to write code in one of the primitive files and reverse engineer it to a template. It need not be fully polished and working, but should provide a starting point. Thoughts?

[jdimeo]

@nikhilnanivadekar I haven't forgotten about this request, but regretfully I'm not sure when I'll have the time to tackle it!

[nikhilnanivadekar]

LGTM. Thanks for your contribution!

[nikhilnanivadekar]

@jdimeo sorry didnt get a chance during the week to work on this. Finally got some time and did a final review. Looks good to me! Thanks a lot for your contribution!
If you get a moment please feel free to blog about the JavadocUtil and/or contributing to the project.
Looking forward to your next PR. Cheers!

[jdimeo]

@nikhilnanivadekar No worries! just wanted to make sure I wasn't the blocker. Happy to help.
Where/how would I submit a blog post draft?

[nikhilnanivadekar]

I meant a blog in your personal space to showcase your experience. May be on Wordpress or even GitHub pages or Medium?
We don’t have an Eclipse Collections blog space, yet. But now that I think about it we should have one. Please don’t wait for us to set that one up.