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 parent readme and changelog #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robin-nitrokey
Copy link
Member

We no longer fork solo2 so there is no need to include their readme and
changelog in our repository anymore.

We no longer fork solo2 so there is no need to include their readme and
changelog in our repository anymore.
@szszszsz
Copy link
Member

szszszsz commented Feb 7, 2022

I wonder how this looks like from the License POV. While MIT seems to be OK with this, Apache seems to have more requirements. If we have stated clearly about upstream in the Readme, then I think we can freely modify the resulting package.

Copy link
Member

@szszszsz szszszsz left a comment

Choose a reason for hiding this comment

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

Please add to Readme a clear note about forking the upstream, and containing code from them in a big part.

@robin-nitrokey
Copy link
Member Author

Good point. We could also add an AUTHORS file that lists all authors. Also, § 4b of the Apache license says:

You must cause any modified files to carry prominent notices stating that You changed the files; and

I think we don’t do that consistently, e. g. here we only list SoloKeys. It might be possible to fix this automatically based on the Git history.

@szszszsz
Copy link
Member

szszszsz commented Feb 8, 2022

Yup. I am in favor of automatic solution, ideally CI run and checked too.
I am sure someone has done that already. If not, Python has packages for git support, and we should manage with that.

@robin-nitrokey
Copy link
Member Author

My idea was to use this in a one-time effort to update the annotations. Your suggestion is to also use a script to keep the annotations up to date, right?

Generally, I like reuse in the CI as a means to make sure that license annotations are present, but of course it cannot check whether they are accurate and up to date.

@szszszsz
Copy link
Member

szszszsz commented Feb 16, 2022

In general I am in favor to automate everything, or at least keep the review procedure of the things we do not have automated, but could bite. Indeed this is more a maintenance project, than giving any actual progress from the user POV. Makes sense to build some tool, if we have similar situation in other projects.
Reuse looks nice!

@szszszsz
Copy link
Member

Is this one still valid?

@robin-nitrokey
Copy link
Member Author

Yes, but we have to fix the attribution issue for the code from Solokeys.

@daringer
Copy link
Collaborator

To move this forward - I would see only one more change:

  • add the fact that this project originally was forked from solokeys/solo to the README.md (contributors section?)

@jans23
Copy link
Member

jans23 commented Dec 11, 2023

@robin-nitrokey Can this be closed?

@robin-nitrokey
Copy link
Member Author

I still think this is something we should do, especially if we plan to split up pynitrokey into a library and a CLI component. Looking at the discussion again, I wonder whether we really need to comply with Apache 2.0. Isn’t it sufficient if we just chose MIT compliance for the import?

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.

4 participants