Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

O11Y-1873: Adjust copyright information in OpenTelemetryDart #62

Merged
merged 3 commits into from
Jun 14, 2022
Merged

O11Y-1873: Adjust copyright information in OpenTelemetryDart #62

merged 3 commits into from
Jun 14, 2022

Conversation

michaelyeager-wf
Copy link
Contributor

@michaelyeager-wf michaelyeager-wf commented Jun 9, 2022 •
edited
Loading

Notes

This PR updates our LICENSE and NOTICE files and adds automation to the Makefile to prepend license information for files which do not have it.

Changes have been partitioned by commit:

  • To only review changes to the Makefile and scripts to insert license information, see this commit.
  • To only review license changes to source files, see this commit

All license changes made to source files were done using the Makefile updates.

Reviewers

@Workiva/observability

Michael Yeager added 2 commits June 9, 2022 16:09
O11Y-1873: Adjust copyright information in OpenTelemetryDart
80ced93 
 - Updates to LICENSE and NOTICE files.
 - Added script to automate enforcing copyright notice in Dart source
   files.
O11Y-1873: Adjust copyright information in OpenTelemetryDart
f62147c 
 - Added and updated copyright notices in source files.
@michaelyeager-wf michaelyeager-wf requested a review from a team as a code owner June 9, 2022 21:20
@rmconsole-wf
Copy link

rmconsole-wf commented Jun 9, 2022 •
edited by rmconsole2-wf
Loading

Merge Requirements Met ✅

Request Rosie to automerge this pull request by including @Workiva/release-management-p in a comment.

General Information

Ticket(s):

Code Review(s): #62
Release Image Tags:

Reviewers: trevorpeterson-wk, changliu-wk, michaelyeager-wf, keruitan-wk

Additional Information

Watchlist Notifications: None
Pull Requests included in release:

	When this pull is merged I will add it to the following release:
	Current version: opentelemetry-dart 0.14.1
	Version after merge: opentelemetry-dart 0.14.1
	Release Ticket(s): O11Y-1871

	This pull is considered a release pull
	The options defined for this repo will be carried out

Note: This is a shortened report. Click here to view Rosie's full evaluation.
Last updated on Tuesday, June 14 11:04 AM CST

@michaelyeager-wf
Copy link
Contributor Author

Verified that automated tools correctly prepend license information. All license changes made to source files were done using the Makefile updates.

QA +1

@aviary3-wk
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@semveraudit-wf
Copy link

semveraudit-wf commented Jun 9, 2022 •
edited
Loading

Public API Changes

No changes to the public API found for commit d5e7cc7

Showing results for d5e7cc7

Powered by semver-audit-service. Please report any problems by filing an issue.
Reported by the dart semver audit client 2.2.2
Browse public API.

Last edited UTC Jun 10 at 15:17:20

keruitan-wk
keruitan-wk previously approved these changes Jun 9, 2022
View reviewed changes
NOTICE Outdated Show resolved Hide resolved
scripts/attach_copyright.sh Outdated Show resolved Hide resolved
trevorpeterson-wk
trevorpeterson-wk previously approved these changes Jun 10, 2022
View reviewed changes
Copy link

@trevorpeterson-wk trevorpeterson-wk left a comment

Choose a reason for hiding this comment

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

Thumbs up'd the newline nits. Other than that, LGTM.

@michaelyeager-wf
Copy link
Contributor Author

Verified that automated tools correctly prepend license information.

QA +1

@michaelyeager-wf
Copy link
Contributor Author

From our legal team:

  • Year/year range should be the year that the code was first created and any year during which changes were made
  • If the code changes over time, we should update the copyright range

This is an optimization we can make now if you're feeling up to it. Otherwise, I'm fine if you want to throw a ticket in the backlog for tech debt.

I think your suggestion makes sense. I'd also like to be able to remove-and-replace the license in the source files with a "master license" so that we can update the content of the license for the entire codebase with one command. However, I'd prefer to look at a non-shell script to accomplish that because it would afford us better flexibility, potential unit testing, and be more maintainable long-term.

All that to say, a tech-debt ticket would be perfect to ensure we're occasionally revisiting tooling here in order to make improvements.

changliu-wk
changliu-wk reviewed Jun 13, 2022
View reviewed changes
lib/src/sdk/proto/opentelemetry/proto/collector/trace/v1/trace_service.pb.dart
@@ -1,3 +1,6 @@
// Copyright 2021-2022 Workiva.
// Licensed under the Apache License, Version 2.0. Please see https://github.com/Workiva/opentelemetry-dart/blob/master/LICENSE for more information

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add copyright header with generated files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Generated files must include a copyright header as well.

@michaelyeager-wf
Copy link
Contributor Author

@tylerrinnan-wf I've created https://jira.atl.workiva.net/browse/O11Y-1891 to address your concerns.

@michaelyeager-wf
Copy link
Contributor Author

@Workiva/release-management-p

rmconsole-wf
rmconsole-wf approved these changes Jun 14, 2022
View reviewed changes
Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rmconsole2-wf rmconsole2-wf merged commit c756634 into Workiva:master Jun 14, 2022
@rmconsole2-wf rmconsole2-wf deleted the myeager-wf/O11Y-1873 branch June 14, 2022 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@changliu-wk changliu-wk changliu-wk approved these changes

@trevorpeterson-wk trevorpeterson-wk trevorpeterson-wk approved these changes

@keruitan-wk keruitan-wk keruitan-wk approved these changes

@rmconsole-wf rmconsole-wf rmconsole-wf approved these changes

Assignees
No one assigned
Labels
Merge Requirements Met RM Ready semver: Patch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@michaelyeager-wf @rmconsole-wf @aviary3-wk @semveraudit-wf @tylerrinnan-wf @keruitan-wk @trevorpeterson-wk @changliu-wk @rmconsole2-wf