apache / incubator-graphar

An open source, standard data file format for graph data storage and retrieval.
https://graphar.apache.org/
Apache License 2.0
195 stars 40 forks source link

[Spark-refactor] Replace List and Map by UnmodifiableXxx #411

Closed Thespica closed 3 months ago

Thespica commented 3 months ago

Proposed changes

Here are the main changes:

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

Related issue #409

acezen commented 3 months ago

As discussion in https://github.com/alibaba/GraphAr/discussions/379#discussioncomment-8860670, seems we had a decision that we should use the UnmodifiableXxx as the immutable object class. The ImmutableXxx would bring a extra dependency that not comply with the "Keep It Simple, Stupid" principle.

acezen commented 3 months ago

JAVA 8 is a very old version that does not support UnmodifiableXxx is reasonable. We can drop the support for JAVA 8 and start from JAVA 11. In the future, the upstream JAVA core would keep bring new feature and JAVA 8 is too old for all JAVA libraries. So I think it's the good time to give up the support of JAVA 8.

Thespica commented 3 months ago

This PR is ready, take a review please @acezen @SemyonSinchenko

acezen commented 3 months ago

Hi, the rename package name from com.alibaba to org.apache is only apply to main. It' not need to change that in this PR. It would bring a lot of changes that are not related to the feature.