eclipse.platform.releng.aggregator icon indicating copy to clipboard operation
eclipse.platform.releng.aggregator copied to clipboard

Automatically check licenses and request review for target updates

Open laeubi opened this issue 10 months ago • 8 comments

If there are updates to the target contents there is some chance we need a review.

This now adds a job that when a PR is created or updated calls the license check workflow together with a review request.

laeubi avatar Jan 19 '25 07:01 laeubi

Yes this will be "silent" at the moment, but my idea is that if one sometimes later rerun the check it is already vetted (so it is actually good), maybe one can make it add a label to the PR (e.g vetted or requires vetting...) as we are all getting already to much mails I think.

laeubi avatar Jan 19 '25 08:01 laeubi

Yes this will be "silent" at the moment, but my idea is that if one sometimes later rerun the check it is already vetted (so it is actually good),

Yes, it's definitely better than before.

maybe one can make it add a label to the PR (e.g vetted or requires vetting...) as we are all getting already to much mails I think.

Yes there are way to many mails, but luckily my mail-program sorts the mails into threads and deleting a two or three mail thread is not really a difference for me 😅. Nevertheless running the license-workflow if a label is present is probably best implemented by generalizing the case for 'automated requests for dependabot PRs' (which unfortunately doesn't work currently due to the permissions model): https://github.com/eclipse-dash/dash-licenses/blob/9b1616198579426e36cc81f8fde2fffcc65d6103/.github/workflows/mavenLicenseCheck.yml#L61-L66

HannesWell avatar Jan 19 '25 08:01 HannesWell

By label I mean, that this job can add a label to the PR when the check was successful, so one can "see" it... but thinking further now one better would want to rerun the license check check if vetting was performed... but this will require a lot more work I fear.

laeubi avatar Jan 19 '25 10:01 laeubi

By label I mean, that this job can add a label to the PR when the check was successful, so one can "see" it... but thinking further now one better would want to rerun the license check check if vetting was performed... but this will require a lot more work I fear.

That would be nice, if possible I would even add a comment to ping the the person responsible for the PR, but as you said that's much more work as it requires a connection from the EF gitlab to the corresponding GH PR. But first of all it would require https://github.com/eclipse-dash/dash-licenses/issues/184, because currently there is no direct link from a Gitlab IP issue to the creating PR.

HannesWell avatar Jan 20 '25 17:01 HannesWell

Is there a reason to not push this one now? Silent request is still better than manual one IMO.

akurtakov avatar Mar 03 '25 07:03 akurtakov

From my side we can try it out ...

laeubi avatar Mar 03 '25 07:03 laeubi

@akurtakov but We would need a new parameter and a condition check here as it is now a shared workflow.

laeubi avatar Mar 03 '25 07:03 laeubi

But you wont get a comment with the pretty-printed results added to the PR created, which I think is especially useful if some artifacts are not vetted and one wants to check the requests status.

I still think not having a comment that adds the created reviews is really sub-optimal as one then has to search the build-logs. A while ago I have adjusted the workflow to request a review automatically for dependabot PRs:

  • https://github.com/eclipse-dash/dash-licenses/pull/220

and it seems to work well:

  • https://github.com/eclipse-dash/dash-licenses/pull/418#issuecomment-2517662335

I think this pattern can be extended to either add an option to permit more bot/user accounts or to add an option that for PRs of a committer (i.e. actors with write access) a review is requested automatically (and the bot should have write access).

HannesWell avatar Mar 03 '25 22:03 HannesWell

What is the status of this one?

akurtakov avatar Jun 18 '25 04:06 akurtakov

As there has been no reply for months I'm closing this one. If/when someone is ready to drive it to completion please reopen and do ti.

akurtakov avatar Oct 03 '25 09:10 akurtakov