eclipse-collections icon indicating copy to clipboard operation
eclipse-collections copied to clipboard

Add missing methods to ImmutableMapFactory

Open kedar-joshi opened this issue 4 years ago • 9 comments

Additional changes include -

  1. Replace usage of newly deprecated methods.
  2. Update interface Javadoc to align with deprecated implementation in (ImmutableMapFactoryImpl#ofMap).

Closes gh-738

Signed-off-by: Kedar Joshi [email protected]

kedar-joshi avatar Mar 08 '20 09:03 kedar-joshi

I think I should also add @Since on new methods. Right ?

kedar-joshi avatar Mar 08 '20 11:03 kedar-joshi

@kedar-joshi thanks for your contribution! Before I review the PR, a little bit of admin stuff so that it is easier for me when I write the release notes. Can you break the additional changes which you have done in separate commits, please? It is fine to have it in this PR, just separate commits, so that I can look at the history easily and compile the release notes. Also, the history remains clean. Yes, please add since tag as well. Please ping me when it is done, I will review it. Cheers! Thanks for helping us out!

nikhilnanivadekar avatar Mar 08 '20 15:03 nikhilnanivadekar

Can you break the additional changes which you have ..

Sure.

kedar-joshi avatar Mar 08 '20 15:03 kedar-joshi

There are build failures. Also the implementation is far easier than what you have right now. Please take a look in the review. Thanks for your contribution!

nikhilnanivadekar avatar Mar 08 '20 17:03 nikhilnanivadekar

Looping in @donraab as he initially opened the issue.

nikhilnanivadekar avatar Mar 08 '20 23:03 nikhilnanivadekar

I see the context of gh-738 now, but the deprecation still doesn't make sense as written. The methods with bad generics are deprecated, but the new methods also have bad generics. There's no version of ofMap that's not deprecated.

I think we should experiment with pulling methods up to a super-interface to drive consistency (in a separate commit) to make this sort of thing enforced by the compiler.

motlin avatar Mar 10 '20 00:03 motlin

Here's an example, to start the conversation: https://github.com/motlin/eclipse-collections/commit/f9397b23d0f2876dce1d184540a5bc1e3a378333

motlin avatar Mar 10 '20 00:03 motlin

And here's a similar example for Maps. This one doesn't compile because the interfaces are truly not compatible. https://github.com/motlin/eclipse-collections/commit/07c81aad52e4f9fb797f5e9a804fac0773447314

motlin avatar Mar 10 '20 00:03 motlin

is this pull request abadoned?

anishlukk123 avatar Oct 26 '23 23:10 anishlukk123