search

eclipse-kuksa / kuksa-actions

Reusable Github actions and workflows used in the Eclipse KUKSA project
Apache License 2.0
1 stars 2 forks source link

Create Dash tickets #8

Closed SebastianSchildt closed 4 months ago

SebastianSchildt commented 4 months ago

This updates the dash action, so that dash tickets can be automatically created if a secret is available.

This should be backwards compatible, but I still suggest tagging a new major version after merging this, just to be sure

If a secret is not given, or not available (in case of PRs from forks) it gracefully falls back to the "checking only"

Currently, even if i.e. the secret is wrong and the tool gives exceptions, the build step still is successful (this seesm more a coincidence, but I think it is good for introducing, until we feel more confident about this. Error messages/Excpetions can still be seen in the steps summary)

Also, Java installation is much faster now as the "apt install" has been replace with an action that uses a cached install

A test repo/various test runs can be seen here https://github.com/SebastianSchildt/dashtest/actions

erikbosch commented 4 months ago

Looking good, but some thoughts/comments just to make sure I understand everything

  1. We already have a token ECLIPSE_GITLAB_API_TOKEN in https://github.com/eclipse-kuksa/.eclipsefdn/blob/main/otterdog/eclipse-kuksa.jsonnet#L59 so to start using this we "just" need to tag this repo after merge, start using @4 and reference the token for every repo where we want Dash checks, right?
  2. For Kuksa we typically always use forks for PRs, but theoretically those that are maintainers could use "local PRs" as well, it is commonly used in for example Velocitas. Do we want to order reviews also for pull requests (if token available)? In one way it makes sense, on the other hand it could result in reviews of versions that actually never will be added
SebastianSchildt commented 4 months ago
  1. We already have a token ECLIPSE_GITLAB_API_TOKEN in https://github.com/eclipse-kuksa/.eclipsefdn/blob/main/otterdog/eclipse-kuksa.jsonnet#L59 so to start using this we "just" need to tag this repo after merge, start using @4 and reference the token for every repo where we want Dash checks, right?

That was exaxctly my plan

  1. For Kuksa we typically always use forks for PRs, but theoretically those that are maintainers could use "local PRs" as well, it is commonly used in for example Velocitas. Do we want to order reviews also for pull requests (if token available)? In one way it makes sense, on the other hand it could result in reviews of versions that actually never will be added

If we add token and otherwise keep our CI, the workflow will run for all PRs. The result is for PRs from external forks it falls back to jsut checking (becasue the token is not accessible) -> No change. For "repo-internal" PRs (which normally we do not like anyway), it would scan and create tickets. Those "external" PRs, after merge to main (i.e. push) when the scan is triggered AGAIN will then create tickets.

It MIGHT be that a "local" PR contains a dependency that will be cleared Eclipse side, that in the end will never be used by us. But then again it helps the next guy. And actually these days I would say 80% of request are cleared completely automatically (i.e. the automated scanner feels confident enough to believe the declared license) and and additonal 10% is flagged by the scanner and an Eclipse IP person takes a look and approves without further interaction. So I think the risk of DOSing the Eclipse process or us is relatively low

SebastianSchildt commented 4 months ago

Also (important) side note: Of course Dash is clever enough to recognize a ticket already exists. I.e. running a buidl 100 times trying to create a ticket 100 times, will only result in one ticket

erikbosch commented 4 months ago

@SebastianSchildt - I approved, you can decide yourself if you want to squash before merge or not. I thinks the commits looks quite nice, so not necessary as I see it.