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

Ns15/remove mime multipart dependency #165

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nsant15
Copy link

@nsant15 nsant15 commented Jan 13, 2023

Removed cryptodependency in textnonce crate by removing mime_multipart dependency and replicating required code with appropriate copyrights.

@nsant15 nsant15 force-pushed the ns15/remove_mime_multipart_dependency branch from a1b418b to b224349 Compare January 13, 2023 14:28
This was done to remove a crypto dependency in the textnonce crate.
Most functionality from mime_multipart has been copied with the relevant
copyright and licensing. mime_multipart functions replicated into
 multipart/related, and moved multipart/related.rs functions into a new
directory.
@nsant15 nsant15 force-pushed the ns15/remove_mime_multipart_dependency branch from cf2fa6f to 0b74852 Compare January 16, 2023 11:12
Copy link
Contributor

@richardwhiuk richardwhiuk left a comment

Choose a reason for hiding this comment

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

You've added a mixture of MIT, Apache and MIT/Apache dual licensed code to a repo which is Apache 2.0 licensed.

In addition, you haven't signed the DCO - presumably because you can't, because this isn't your own work.

As such, it's going to be difficult to accept this code in the current form.

In addition, this code represents a breaking change.

@@ -1,38 +0,0 @@
//! Helper functions for multipart/related support
Copy link
Contributor

Choose a reason for hiding this comment

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

Where has this file gone?

Copy link
Author

Choose a reason for hiding this comment

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

I have moved the functions into the mod.rs file of the new directory (as mentioned in the commit message). I thought this would have the same effect as having the file as it was before.

@nsant15
Copy link
Author

nsant15 commented Jan 17, 2023

You've added a mixture of MIT, Apache and MIT/Apache dual licensed code to a repo which is Apache 2.0 licensed.

In addition, you haven't signed the DCO - presumably because you can't, because this isn't your own work.

As such, it's going to be difficult to accept this code in the current form.

In addition, this code represents a breaking change.

I did the licensing in this way as that is how I interpreted how to do it from @james-jra's suggestions. How would you recommend rectifying this if it is a problem?

I thought that the DCO could only be signed after reviewing.

Is the breaking change preventable? The replication of the crate functions is needed to remove the crypto dependency on the textnonce crate.

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