Skip to content

Conversation

@HeartLinked
Copy link
Contributor

@HeartLinked HeartLinked commented Sep 23, 2025

Refines the LICENSE and NOTICE files to ensure full compliance with Apache Software Foundation (ASF) guidelines.https://infra.apache.org/licensing-howto.html

  • Simplify NOTICE File: Dependencies with permissive licenses (such as MIT, zlib, and Public Domain) have been removed from the NOTICE file. According to ASF policy, the copyright notices embedded within these types of licenses do not need to be duplicated in the project's NOTICE file.

  • Reformat LICENSE File: The appendix of the LICENSE file, which lists third-party dependencies, has been reformatted for clarity and consistency. The new style is based on the Apache Iceberg repository's LICENSE file. https://github.com/apache/iceberg/blob/main/LICENSE

  • Add cpr license for rest catalog.

@HeartLinked
Copy link
Contributor Author

@jbonofre Could you help review this?

@jbonofre
Copy link
Member

@HeartLinked absolutely. Let me take a look. Happy to help 😃

@jbonofre
Copy link
Member

The NOTICE file is better and correct. Thanks for that.

Imho, the LICENSE file is not fully correct because:

  1. The MIT/BSD license content is missing (I don't see a separate folder containing these licenses, and the license content is not inline).
  2. It's not clear to me what do you mean by "bundle". Can you please clarify ?

@HeartLinked
Copy link
Contributor Author

HeartLinked commented Sep 24, 2025

The NOTICE file is better and correct. Thanks for that.

Imho, the LICENSE file is not fully correct because:

  1. The MIT/BSD license content is missing (I don't see a separate folder containing these licenses, and the license content is not inline).
  2. It's not clear to me what do you mean by "bundle". Can you please clarify ?

Thanks for your review! Actually, our project is distributed as a source package that users compile themselves. We do not copy source files from zlib, spdlog, cpr, etc. Instead, we directly use system installed them or vendor it by building the downloaded tarball. Previously, our LICENSE file contained sentences like "3rdparty dependency spdlog is statically linked in certain binary distributions". Based on this, I had mistakenly concluded that these dependencies are "bundled".

Following our discussion, I have now removed all related third-party license content from the file. Do you think this revised version is more accurate? @jbonofre

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's correct now. Thanks !

LICENSE Outdated
https://github.com/apache/arrow/blob/main/cpp/src/arrow/util/decimal.cc
* checked_cast utility in src/iceberg/util/checked_cast.h.
* visit_type utility in src/iceberg/util/visit_type.h.
* Decimal128 implementation details in src/iceberg/util/decimal files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: cmake_modules/IcebergBuildUtils.cmake and cmake_modules/IcebergThirdpartyToolchain.cmake are also adapted from arrow-cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added these. :)

@Fokko
Copy link
Contributor

Fokko commented Sep 25, 2025

Thanks @HeartLinked for the follow up and thanks everyone for the review 🙌

@Fokko Fokko merged commit ab0662f into apache:main Sep 29, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants