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

Implement support for SPDX-FileContributor #669

Merged
merged 5 commits into from
May 11, 2023

Conversation

sgaist
Copy link
Contributor

@sgaist sgaist commented Feb 7, 2023

This patch adds a new parameter that allows to list one or more contributors which will generate the corresponding list of fields in the header.

It refactors SpdxInfo into a dataclass so that it can be used with an optional list of contributors in a simple fashion.

Fixes #615

@carmenbianca
Copy link
Member

Hi @sgaist ! Thanks for this PR. It looks awesome. @linozen or me will review this quite soon, because we needed something just like this to implement another feature in #654.

We will also be dropping Python 3.6 support basically now-ish. If Python 3.6 is blocking you, please do ignore it.

@sgaist
Copy link
Contributor Author

sgaist commented Feb 10, 2023

Hi @carmenbianca ! You're welcome. There might be some work to be done on the test side as I only re-implemented two that were obvious good candidate but it might worth refactoring a bit more.

@sisch
Copy link
Contributor

sisch commented Apr 5, 2023

@sgaist For the moment we decided to finish the json PR first and adapt to dataclass in your separate PR. The code locations we needed to change can be found in commit 6b2b3b9ca5fa947a6ced97fa6e7e7fd967f5ccd5. So the best strategy to finish this PR (#669) in my eyes is to merge main after release of PR #654 and add the license_path field to the dataclass.

@linozen linozen mentioned this pull request Apr 5, 2023
@sgaist
Copy link
Contributor Author

sgaist commented Apr 5, 2023

@sisch, ok, i'll wait for #654 to be merged and refactor this PR.

One related question: do you prefer that I merge main or rebase my work on it ?

@sisch
Copy link
Contributor

sisch commented Apr 5, 2023

@sgaist I'm afraid I need to pass this question to @linozen: do you know if rebase or merge is preferred?
Personally I think rebase in this specific case is better, but it's not my call, I'm not a maintainer ;)

@mxmehl
Copy link
Member

mxmehl commented Apr 5, 2023

@sisch, ok, i'll wait for #654 to be merged and refactor this PR.

Thank you for your patience!

One related question: do you prefer that I merge main or rebase my work on it ?

In PRs, we usually prefer rebases. In this specific case I personally would also prefer a rebase unless a merge is much easier for you.

@sgaist
Copy link
Contributor Author

sgaist commented Apr 5, 2023

Rebase is perfect, that is my preferred way of working.

I'll keep an eye on #654 to keep things rolling.

@carmenbianca
Copy link
Member

I made some small changes and rebased this on top of main. I intend to merge this before #654 because that PR would heavily benefit from the dataclass refactoring.

@sgaist sgaist force-pushed the implement_contributor_field branch from e5199aa to 601b648 Compare April 17, 2023 12:18
@nicorikken nicorikken added blocked This issue/pull request is blocked by anoter and removed blocked This issue/pull request is blocked by anoter labels Apr 27, 2023
@nicorikken
Copy link
Member

Something to take note of, is that it is not possible to only provide the --contributor parameter:

$ reuse annotate --contributor "A Person" --recusvie myrepo/
 ... (usage info)
reuse annotate: error: option --copyright or --license is required

This can be a design choice, but I can imagine scenario's where a user would like to use a githook to add themselves as contributor, regardless of the copyright or license information. What do you think?

@sgaist
Copy link
Contributor Author

sgaist commented Apr 27, 2023

@nicorikken Good catch ! It's how I thought I had implemented it but did all the tests in the context of making it part of adding either of the other two fields.

sgaist and others added 4 commits April 27, 2023 14:29
This patch adds a new parameter that allows to list one or more contributors
which will generate the corresponding list of fields in the header.

It refactors SpdxInfo into a dataclass so that it can be used with an optional
list of contributors in a simple fashion.
…better

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@sgaist sgaist force-pushed the implement_contributor_field branch from 601b648 to 5124936 Compare April 27, 2023 12:29
src/reuse/__init__.py Outdated Show resolved Hide resolved
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@linozen linozen merged commit 8d8389b into fsfe:main May 11, 2023
@sgaist sgaist deleted the implement_contributor_field branch May 16, 2023 14:02
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.

feature request: add support for contributors to addHeader
6 participants