Skip to content

Conversation

@vrubezhny
Copy link
Contributor

No description provided.

@vrubezhny vrubezhny marked this pull request as draft November 23, 2022 22:17
@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch 2 times, most recently from 0a93d98 to af146d9 Compare November 23, 2022 22:43
@vrubezhny vrubezhny changed the title Use GitLub Token for License Check Use GitHub Token for License Check Nov 23, 2022
@vrubezhny vrubezhny changed the title Use GitHub Token for License Check Use GitLab Token for License Check Nov 23, 2022
- name: License check
run: |
mvn -U -V -e -B -ntp org.eclipse.dash:license-tool-plugin:license-check --file pom.xml
mvn -U -V -e -B -ntp org.eclipse.dash:license-tool-plugin:license-check --file pom.xml -Ddash.projectId=tools.wildwebdeveloper -Ddash.iplab.token=${{ secrets.GITLAB_API_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's worth also using this change to enable the common license check action, as done in Tycho and m2e: https://github.com/eclipse-tycho/tycho/blob/master/.github/workflows/licensecheck.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.
It's just that I'm not sure if the token really works... The dash.iplab.token argument was accepted for sure because the Dash Licence plugin reported on that IPTeam issues are already exist:

[INFO] A review request already exists https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/5062 .
...
[INFO] A review request already exists https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/5063 .

But it never tried to create an issue yet, so I don't know yet if the argument value is really accepted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I'm not sure about is secret name I've used... The guide and examples (like https://github.com/eclipse-tycho/tycho/blob/master/.github/workflows/licensecheck.yml) stay on adding a project name to the token (like TYCHO_GITLAB_API_TOKEN, for example), while Frederic states he created GITLAB_API_TOKEN (NOT prefixed with the WWD project name). Probably I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, we probably have to add the token usage to NPM dependencies check in #967

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from af146d9 to 2f54e4b Compare November 24, 2022 11:59
@vrubezhny
Copy link
Contributor Author

vrubezhny commented Nov 24, 2022

@mickaelistria The common license check added like this:

  call-license-check:
    uses: eclipse/dash-licenses/.github/workflows/mavenLicenseCheck.yml@master
    with:
      projectId: tools.wildwebdeveloper
    secrets:
      gitlabAPIToken: ${{ secrets.GITLAB_API_TOKEN }}

finds the same two not resolved artifacts (so, it looks like we're double checking them) but with two differences:

  • it fails at the end of the check (because of those two not resolved artifacts)
  • it doesn't report on any try to create a review request.

There is require-review input in https://github.com/eclipse/dash-licenses/blob/master/.github/actions/maven-license-check-action/action.yml that is to be used to force a review requesting, but I don't see any usages of this input in https://github.com/eclipse-tycho/tycho/blob/master/.github/workflows/licensecheck.yml ...
On other side, I saw the IPTeam requests created by tycho - was they created by "ommenting with /request-license-review"? Or there was require-review input set in https://github.com/eclipse-tycho/tycho/blob/master/.github/workflows/licensecheck.yml but later removed from the code?

@akurtakov
Copy link
Contributor

Your command is not hooked to anything in licensecheck.yml thus nothign will happen for /request-license-review . It is handled at https://github.com/eclipse/dash-licenses/blob/master/.github/actions/maven-license-check-action/action.yml#L29 so using it will have effect.

@vrubezhny
Copy link
Contributor Author

Your command is not hooked to anything in licensecheck.yml thus nothign will happen for /request-license-review . It is handled at https://github.com/eclipse/dash-licenses/blob/master/.github/actions/maven-license-check-action/action.yml#L29 so using it will have effect.

@akurtakov aren't we using it when we invoke the following job at https://github.com/eclipse/wildwebdeveloper/pull/977/files#diff-214dc48999f71aa20bbf3110511b812571737db7a5da2e71a94d16c55255c08fR40:

 call-license-check:
    uses: eclipse/dash-licenses/.github/workflows/mavenLicenseCheck.yml@master
    ...

?

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from 2f54e4b to 5f6d4ea Compare November 24, 2022 14:32
@mickaelistria
Copy link
Contributor

so, it looks like we're double checking them

The idea is to replace the custom mvn step by invoking the GitHub workflow. As to /request-license-review, it will most likely only work when the necessary config is merged to master.

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from 5f6d4ea to 028efaa Compare November 24, 2022 15:12
@vrubezhny
Copy link
Contributor Author

so, it looks like we're double checking them

The idea is to replace the custom mvn step by invoking the GitHub workflow. As to /request-license-review, it will most likely only work when the necessary config is merged to master.

So, we should remove the 'build` job as result, right?

How can I make sure that the dependency list generated in the build job equals to the one that is generated in "the GitHub workflow"? I mean, currently they produce quite the same list of not vetted artifacts, but probably the best way would be comparing the generated lists of dependencies to be used by License Check?

@mickaelistria
Copy link
Contributor

So, we should remove the 'build` job as result, right?

Yes, the 'build' job would "only" build and test; and reviewing the license would be another workflow (like it's done in Tycho and m2e). This becomes easier to maintain and review.

How can I make sure that the dependency list generated in the build job equals to the one that is generated in "the GitHub workflow"?

You don't have to make sure of that, but only to trust the GitHub workflow (Which is becoming the standard approach recommended by CBI).

@vrubezhny
Copy link
Contributor Author

/request-license-review

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from 028efaa to 40ca81c Compare November 24, 2022 15:39
@vrubezhny
Copy link
Contributor Author

cc @wildwebdeveloper-bot

@vrubezhny
Copy link
Contributor Author

/request-license-review

@mickaelistria
Copy link
Contributor

Again "/request-license-review" is not likely to work before the change is merged. A 1st iteration is to make the GitHub workflow use the CBI one; then we'll see if /request-license-review can work with it.

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from 40ca81c to 3e72339 Compare November 24, 2022 15:52
@vrubezhny
Copy link
Contributor Author

@mickaelistria OK. Then I'm going to merge the change that replaces the build job with the call-license-check that invokes the dash-licenses's mavenLicenseCheck.yml and enables the invocation on issue-comment event

@vrubezhny vrubezhny force-pushed the fixLicenseCheckUseGithubToken branch from 3e72339 to 0ed7420 Compare November 24, 2022 16:16
@vrubezhny vrubezhny marked this pull request as ready for review November 24, 2022 16:17
@vrubezhny vrubezhny merged commit e412b90 into eclipse-wildwebdeveloper:master Nov 24, 2022
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.

3 participants