Skip to content

Allow configuring if existing headers should be skipped in license check#303

Merged
findepi merged 1 commit intoairlift:masterfrom
hashhar:hashhar/allow-configure-license-skip-existing-headers
Mar 21, 2022
Merged

Allow configuring if existing headers should be skipped in license check#303
findepi merged 1 commit intoairlift:masterfrom
hashhar:hashhar/allow-configure-license-skip-existing-headers

Conversation

@hashhar
Copy link
Copy Markdown
Contributor

@hashhar hashhar commented Mar 18, 2022

All credit to @findinpath for finding this.

See for source trinodb/trino#11563 (comment)

cc: @findinpath @findepi

<configuration>
<skip>${air.check.skip-license}</skip>
<skipExistingHeaders>true</skipExistingHeaders>
<skipExistingHeaders>${air.license.skip-existing-headers}</skipExistingHeaders>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not always false?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(it's true since ever (d71d25e))

Copy link
Copy Markdown
Contributor Author

@hashhar hashhar Mar 18, 2022

Choose a reason for hiding this comment

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

Just to preserve compatibility. IMO it should be false.
A case where this can be important is if you have mixed license files and want to preserve original licenses.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@electrum can you please chime in here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let’s make this false, since that is the better default. Any projects that need to skip can set the property when updating.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated default to true and adjusted CHANGES entry accordingly.

This behaviour can be controlled by the
air.license.skip-existing-headers property.
@findepi findepi merged commit 77084d7 into airlift:master Mar 21, 2022
@hashhar hashhar deleted the hashhar/allow-configure-license-skip-existing-headers branch March 21, 2022 10:18
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