Hi Justin, please see the inline comments.

Justin Mclean <jus...@classsoftware.com> 于2024年9月1日周日 19:39写道:

> Hi,
>
> -1 (binding) as some files are misisng incubating in their names, but if
> those files are renamed, then +1 (binding)
>
> Several LICENSE and NOTICE files need updating, but that’s OK due to the
> WIP disclaimer.
>
> Testing on mocOS I checked:
> - Not all artifact names include incubating in them, e.g.
> apache_gravitino-0.6.0.tar.gz. This needs to be fixed, but there is no need
> for a new RC or vote, just rename the files.
>

For Python release "apache_gravitino-0.6.0.tar.gz" version, as far as I
know, Python doesn't support version like "0.6.0-incubating", so I only
keep the 0.6.0 delibrately. I also refer to some other incubating projects
like HugeGraph (https://pypi.org/project/hugegraph-python/#history), seems
they also don't add the "incubating" in their version.


> - Signatures and hashes are fine.
> - DISCLAIMER (WIP), NOTICE and LICENSE exist in all artifacts.
> - The LICENSE and NOTICE is incorrect for apache_gravitino-0.6.0.tar.gz,
> same with the licenses directory. The LICENSE and NOTICE files need to
> reflect what is in the release artifact, and it seems the binary LICENSE
> and NOTICE files for the Java release have been copied here. In this case
> of the stock LICENSE file, adding "MIT License Copyright (c) 2016 Dhamu” is
> all that is needed. BTW do we know what project this code comes from?
>

Can you please help to work on this, I don't think I have enough time to
figure out all these version things one by one.


> - Similarly, the LICENSE and NOTICE files for
> gravitino-iceberg-rest-server-0.6.0-incubating-bin and
> gravitino-trino-connector-0.6.0-incubating need to be updated to reflect
> what is in them.
>

Also for this, please help on this.


> - The main gravitino LICENSE is missing a license for TCPDS. I couldn't
> find out this data was licenced from a quick search. Do we know how the
> data is licensed?
>

TPCDS data is generated by its tool, they're the random data (will be
varied in each execution). I don't find the license of TPC-DS tool, and not
sure about the data if it is OK with ASF license.


> - all ASF files have the correct header, although perhaps it could be
> added to some XML files.
>

Can you please create an issue so we can add this?


> - No unexpected binary files in the source release
> - Compile fails due to rat issues, but can compile from source (and test
> pass) with the addition of -x rat
>

The local compile works for me. I'm not so sure about which file has the
issues.


>
> Other minor issues:
> - The KEY file needs to be in
> https://dist.apache.org/repos/dist/release/incubator/gravitino/


It will be there when we have an official release.


>
> - I think gravitino-trino-connector-0.6.0-incubating.tar.gz should include
> a “-bin” in the name as it’s not source code. Again, this can be renamed
> without a new RC or vote.
>

Typically for trino connector, we don't add "-bin" in the name as well as
other connectors, it's a bit stranger as it is only a plugin, not a
standalone service.


> - Is the source code for gravitino-trino-connector-0.6.0-incubating and
> gravitino-iceberg-rest-server-0.6.0-incubating included in the release? (I
> am asking this in case this comes up in the IPMC vote).
>

They're all included in the Gravitino src package.


> - The rat test fails when building the source. The files it fails on are
> acceptable, except for possibly some XML files.
>

Please point out the problems and create an issue. I don't see the issue in
my local build.


> - Do the “.github” and “.gitattributes” files need to be in the source
> release?
>

I think they're also part of the source code, they can be shipped with
source release. If it is not required, we can do this in the next release.


> - The Python client README.md build instruction needs to be updated as
> they assume it part of the main gravitino repo
>
>
Python README.md already reflects the Python things including how to build
Python package, can you please point out what specific issue it is?



>
> Kind Regards,
> Justin

Reply via email to