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

Remove image license files #262

Merged
merged 4 commits into from Dec 16, 2020
Merged

Remove image license files #262

merged 4 commits into from Dec 16, 2020

Conversation

aaronayres35
Copy link
Contributor

fixes #236

The issue was motivated by this comment: #233 (comment)

Note, in making this PR I did noticed there is still one images directory in apptools: https://github.com/enthought/apptools/tree/master/apptools/logger/plugin/view/images
but it has its own image_LICENSE.txt file: https://github.com/enthought/apptools/blob/master/apptools/logger/plugin/view/images/image_LICENSE.txt

This PR also removes the deleted files from the MANIFEST.in file.

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

@rahulporuri
Copy link
Contributor

I'd actually recommend moving the internal image license file - https://github.com/enthought/apptools/blob/master/apptools/logger/plugin/view/images/image_LICENSE.txt - to the top level, and including that in the manifest file. Also, I don't know what image_LICENSE_CP.txt is and I don't see that in the repository. That will also need to be added, moved to the top level (root git directory) and included in the manifest.

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

Mostly LGTM

image_LICENSE.txt Outdated Show resolved Hide resolved
image_LICENSE.txt Outdated Show resolved Hide resolved
@rahulporuri
Copy link
Contributor

@aaronayres35 i'm not sure where you got the image_LICENSE_CP.txt file.

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
@aaronayres35
Copy link
Contributor Author

@aaronayres35 i'm not sure where you got the image_LICENSE_CP.txt file.

@rahulporuri I found it in envisage and assumed it was supposed to be the same file (that assumption should definitely be confirmed though!)

@aaronayres35 aaronayres35 merged commit 7c68a79 into master Dec 16, 2020
@aaronayres35 aaronayres35 deleted the remove-image-license-files branch December 16, 2020 20:52
@aaronayres35 aaronayres35 added the need backport to 5.0 PRs that need to be backported to maint/5.0 label Dec 16, 2020
aaronayres35 added a commit that referenced this pull request Dec 16, 2020
* remove image license files

* move internal image license file to root directory

* add image_LICENSE_CP.txt and include it in manifest

* Apply suggestions from code review

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
aaronayres35 added a commit that referenced this pull request Dec 17, 2020
* Make sure optional dependencies are optional for tests (#260)

* make sure optional dependencies are optional for tests

* add a news fragment

* apply suggestion from code review

* Add extras_require to setup.py for optional dependencies (#257)

* add optional dependencies for specific cub packages to extras_require

* add details to README about optional dependencies

* list io imports as standard library imports

* add prefrences/configobj to extras_rrequire as well (leave configobj in install_requires too)

* add specific install instructions

* formatting

* remove unneeded back ticks

* move install instructions into a separate section

* add news fragment

* add PR number to news fragment

* Remove image license files (#262)

* remove image license files

* move internal image license file to root directory

* add image_LICENSE_CP.txt and include it in manifest

* Apply suggestions from code review

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

* Add a new category for build system related changes (#261)

* update changelog

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
Co-authored-by: Kit Choi <kitchoi@users.noreply.github.com>
aaronayres35 added a commit that referenced this pull request Dec 17, 2020
* Make sure optional dependencies are optional for tests (#260)

* make sure optional dependencies are optional for tests

* add a news fragment

* apply suggestion from code review

* Add extras_require to setup.py for optional dependencies (#257)

* add optional dependencies for specific cub packages to extras_require

* add details to README about optional dependencies

* list io imports as standard library imports

* add prefrences/configobj to extras_rrequire as well (leave configobj in install_requires too)

* add specific install instructions

* formatting

* remove unneeded back ticks

* move install instructions into a separate section

* add news fragment

* add PR number to news fragment

* Remove image license files (#262)

* remove image license files

* move internal image license file to root directory

* add image_LICENSE_CP.txt and include it in manifest

* Apply suggestions from code review

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>

* Add a new category for build system related changes (#261)

* update changelog

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
Co-authored-by: Kit Choi <kitchoi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need backport to 5.0 PRs that need to be backported to maint/5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

image_LICENSE.txt needs updating
2 participants