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

Add workflow to lint 3rd party repositories #783

Merged
merged 9 commits into from
Jul 6, 2023
Merged

Conversation

mxmehl
Copy link
Member

@mxmehl mxmehl commented Jun 22, 2023

Fixes #782

@mxmehl mxmehl force-pushed the ci-3rd-party-projects branch 2 times, most recently from 77df507 to b633486 Compare June 22, 2023 10:09
.github/workflows/3rd_party_lint.yaml Outdated Show resolved Hide resolved
.github/workflows/3rd_party_lint.yaml Outdated Show resolved Hide resolved
@mxmehl mxmehl force-pushed the ci-3rd-party-projects branch 3 times, most recently from 04f7178 to 80aa43b Compare June 22, 2023 12:45
@mxmehl mxmehl requested a review from carmenbianca June 22, 2023 13:05
@mxmehl mxmehl marked this pull request as ready for review June 22, 2023 13:06
@mxmehl
Copy link
Member Author

mxmehl commented Jun 22, 2023

So I'm not sure about the poetry lockfile. I installed poetry 1.1.x as described in the docs but it changed a lot of things in the lockfile that weren't there before, although the newly added dependency didn't introduce them, AFAIK. Not sure how you handled that before.

Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

I did some refactoring.

  • I reintroduced the matrix so that you can clearly see which repo failed in the CI.
    • There is some duplication here between the workflow file and the Python file to make this work. It's probably fine.
  • This requires Poetry 1.2 to make this work because of poetry run does not relay exit code python-poetry/poetry#2369
    • I will work on making some changes to switch to Poetry 1.2 later.
  • Probably some more.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
This is needed because Poetry 1.2 is the version that introduced return
codes for scripts generated by Poetry, and we depend on the return code
of `reuse`. See <python-poetry/poetry#2369>.

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@carmenbianca
Copy link
Member

https://invent.kde.org/sysadmin/ci-utilities/raw/master/gitlab-templates/reuse-lint.yml

Apparently KDE removes the po files before doing the check.

@mxmehl
Copy link
Member Author

mxmehl commented Jun 26, 2023

https://invent.kde.org/sysadmin/ci-utilities/raw/master/gitlab-templates/reuse-lint.yml

Apparently KDE removes the po files before doing the check.

I'm not sure how we shall proceed here. I wouldn't want to integrate dirty hacks that third parties have made in order to become REUSE compliant.

@carmenbianca
Copy link
Member

Should we communicate with the KDE people to see if they can use .reuse/dep5 to deal with po files?

In any case I would really like to include a KDE project here. I'm happy to implement a hacky workaround to make this test run. Not in reuse itself; just in the test script.

@carmenbianca carmenbianca self-requested a review June 26, 2023 10:55
@mxmehl
Copy link
Member Author

mxmehl commented Jun 27, 2023

Should we communicate with the KDE people to see if they can use .reuse/dep5 to deal with po files?

Yes, that would be good. Perhaps we can ask @cordlandwehr?

In any case I would really like to include a KDE project here. I'm happy to implement a hacky workaround to make this test run. Not in reuse itself; just in the test script.

How about we merge this PR without KDE first, and create an issue/PR for the KDE one?

@cordlandwehr
Copy link

cordlandwehr commented Jun 29, 2023

Should we communicate with the KDE people to see if they can use .reuse/dep5 to deal with po files?

Yes, that would be good. Perhaps we can ask @cordlandwehr?

In any case I would really like to include a KDE project here. I'm happy to implement a hacky workaround to make this test run. Not in reuse itself; just in the test script.

How about we merge this PR without KDE first, and create an issue/PR for the KDE one?

@mxmehl I think our problem in KDE is more a procedure one than a technical one. The story is that many applications were transformed to be REUSE complient. At that time translations lived outside the repository. About a year ago the translation workflow was changed to move translations into the individual repositories. Yet, most of them are not yet REUSE complient and that will take time for the reason that translation files are modified by external tools (note: these tools also write and update copyright information) which are not yet ready to output SPDX complient files.
So, we have a repository that is complient in all parts (and thus shall be covered by the reuse tests) and one folder that is in a kind of a staging state and thus need to be excluded for the time being, because tests for source code shall be active.
Thinking from test systems maybe an option like EXPECTED_FAIL for folders would be helpful here.
The goal in my opinion is to get the PO folders fully REUSE complient though and I do not see any need for dep5, because each file already contains a copyright header.

@carmenbianca
Copy link
Member

The goal in my opinion is to get the PO folders fully REUSE complient though and I do not see any need for dep5, because each file already contains a copyright header.

That's awesome. It depends on the workflow, I think. We use (hosted) weblate, which doesn't add copyright headers.

But if you can implement this without dep5, all the better.

@mxmehl How do you want to proceed? Write a special case for KDE, or create an issue and wait?

@mxmehl
Copy link
Member Author

mxmehl commented Jun 29, 2023

@mxmehl How do you want to proceed? Write a special case for KDE, or create an issue and wait?

I think it's worth to merge this PR (and remove KDE beforehand) so we can make use of the tests in the upcoming PRs. But yes, please create an issue to add KDE (and perhaps also other repos/orgs), it's worth it :)

@linozen linozen merged commit 0425048 into main Jul 6, 2023
18 checks passed
@linozen linozen deleted the ci-3rd-party-projects branch July 6, 2023 08:48
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.

Check 3rd party repos before release or as regular CI job
4 participants