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.

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

Update copyright headers #459

Merged
merged 7 commits into from
Apr 4, 2024
Merged

Conversation

mmahsereci
Copy link
Contributor

Issue #, if available:

Description of changes:

Test PR to see how headers are looking like.

@apaleyes I can change the text as needed. This is just to see if things work. All files before "2020-10-26" should have two copyright notices in the header and the ones created after only one.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

codecov bot commented Mar 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.09%. Comparing base (e42c5bb) to head (e139c8c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
+ Coverage   89.04%   89.09%   +0.04%     
==========================================
  Files         137      137              
  Lines        4820     4842      +22     
  Branches      547      547              
==========================================
+ Hits         4292     4314      +22     
  Misses        403      403              
  Partials      125      125              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mmahsereci
Copy link
Contributor Author

There may be a compatibility problem with pytest and pytest-lazy-fixtures TvoroG/pytest-lazy-fixture#65 which fails some tests.

Copy link
Collaborator

@apaleyes apaleyes left a comment

Choose a reason for hiding this comment

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

This looks very good! I found only one inconsistency, see below. I am also very happy to look into the tests problem and have it fixed. Thanks, @mmahsereci !

@@ -1,4 +1,4 @@
# Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
# Copyright 2024 The Emukit Authors. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's nice to see! The script certainly worked well here, as this file dates to 2022. So it was given incorrect header when created, and script handled this fine. Great!

setup.py Outdated
Comment on lines 1 to 3
# Copyright 2024 The Emukit Authors. All Rights Reserved.
#
# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's interesting! In most files the headers carry licence identifier, but sometimes, like here, they don't. Do you know why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 3 files or so that had different headers with an extended license mention (It may not be visible in the diff). This file (setup.py) is one of them. Here is the unchanged file for comparison. Can you have a look at the original and updated version and comment on how you think we should handle these `special' files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say let's make all headers look the same, so that there are no special files going forward

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll change those to match the other headers.

I just saw that some of the non-py files have no headers (e.g., pyproject.toml) shall I add headers to those as well? Also can you have a look at the NOTICE file? I just added one line there.

@mmahsereci
Copy link
Contributor Author

mmahsereci commented Apr 4, 2024

I changed the years to year ranges in the copyright statements @apaleyes .

As mentioned offline, I am merging this for now.

@mmahsereci mmahsereci merged commit 97188a9 into EmuKit:main Apr 4, 2024
7 of 10 checks passed
@mmahsereci mmahsereci deleted the mm-license-headers branch April 4, 2024 18:40
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.

2 participants