Skip to content

Conversation

@pingtimeout
Copy link
Contributor

Note for reviewers: the ASF guidelines says that "LICENSE and NOTICE files belong at the top level of the source tree". But I think it would be akward if there was a LICENCE file at the root of the polaris-tools repository but not in any of the tools that are packaged independently.

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.

As I guess it's for source distribution, it can go at the root level.

Copy link

@flyrain flyrain left a comment

Choose a reason for hiding this comment

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

I agreed with @jbonofre that one license file is good for src distribution, but we will need separate ones if we want to distribute them separately, like a binary distribution for the migrator.

@pingtimeout
Copy link
Contributor Author

@flyrain should we revisit this when we start releasing the tools?

distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
Copy link
Member

Choose a reason for hiding this comment

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

I think this LICENSE is incomplete (assuming we plan a source distribution for polaris-tools):

  • it should mention gradle wrapper present in the subfolders
  • it should mention that catalog migrator is coming from Nessie

Also the NOTICE is missing in this regard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The NOTICE file will come with a separate PR, as I am still figuring out what should be in there. Regarding the LICENSE file, do you mean that it should be the union of all the LICENSE files that are in subdirectories? If that's the case, I am in favor of not having such a file at the root of the repository as the value is not clear.

Copy link
Member

Choose a reason for hiding this comment

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

No I mean we would need two kinds of license: one for source distribution, one for binary distribution. The content often differs.

Copy link
Member

Choose a reason for hiding this comment

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

And also yes the root license should be valid for all subfolders (from source distribution standpoint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I am even more confused :D

AFAICT, the current LICENSE file that exists in the catalog migrator is not valid, as it does neither include the license of the plugins nor of the dependencies. So I am not sure whether it would be for the source or for the binary distribution.

I suspect that this source vs. distribution distinction is why we have a LICENSE file at the root of the polaris repository as well as one at runtime/distribution/LICENSE.

Could you confirm that this assumption is correct? As far as I can tell, https://infra.apache.org/licensing-howto.html only applies to bundled dependencies, i.e. for the binary distribution LICENSE (and NOTICE).

Copy link
Member

Choose a reason for hiding this comment

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

Not only. It's also for code copy

Copy link
Member

Choose a reason for hiding this comment

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

Not only. It's also for code copy. Let me update this pr you will see.

@flyrain
Copy link

flyrain commented Sep 23, 2025

Yes, we could revisit once we are releasing the binary package.

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.

3 participants