Skip to content

Add NOTICE.txt file at the root level of a package#151

Merged
rw-access merged 4 commits intomasterfrom
add-notice-file
Mar 17, 2021
Merged

Add NOTICE.txt file at the root level of a package#151
rw-access merged 4 commits intomasterfrom
add-notice-file

Conversation

@rw-access
Copy link
Copy Markdown
Contributor

@rw-access rw-access commented Mar 17, 2021

What does this PR do?

Closes #149

Adds a new optional NOTICE.txt file to a package. This isn't an asset but is the NOTICE.txt for the whole package.

Why is it important?

Some assets can be derived from licensed material and require us to propogate NOTICE.txt. The current use-case is for the security_rule asset type, because some rules (basically query + metadata) have been derived from license materials.

We will need a way to present this information to the user. I imagine the specific integration page is sufficient.

Checklist

Related issues

#142 (PR)
elastic/package-storage#843 (PR)

@rw-access rw-access added the enhancement New feature or request label Mar 17, 2021
@rw-access rw-access requested review from mtojek and ruflin March 17, 2021 11:18
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Mar 17, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #151 updated

  • Start Time: 2021-03-17T15:01:36.387+0000

  • Duration: 6 min 21 sec

  • Commit: 93a6e57

Trends 🧪

Image of Build Times

Copy link
Copy Markdown
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM. But lets way on a 👍 from @ycombinator or @mtojek .

@rw-access As soon as we get it in, could you follow up with an issue in Kibana to make sure we also add the logic in Kibana to expose it. Otherwise it will be pretty hard for any user to find it.

$ref: "./changelog.spec.yml"
- description: The package's NOTICE file
type: file
contentMediaType: "text/plain"
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.

Is it necessary to define the content type here provided that no action is taken based on this? Did you try without this definition?

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.

I just made an assumption that this was necessary. I'm not intimately familiar with the details but am glad to remove if that is preferred.

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.

I'm okay leaving this in. We may not take any action on this content type today but if we do in the future, the NOTICE file would automatically get covered.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

@rw-access
Copy link
Copy Markdown
Contributor Author

Thanks @ycombinator and @ruflin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Change Proposal] Optional NOTICE.txt file for integrations

5 participants