search

eclipse-uprotocol / up-spec

uProtocol Specifications
Apache License 2.0
32 stars 25 forks source link

verify-pr CI job blocks merging of PRs #143

Closed sophokles73 closed 4 months ago

sophokles73 commented 4 months ago

A PR for the up-spec repository which does not contain changes proto3 files cannot be merged.

This is because the verify-pr job that is used for checking if the Core API protos can still be transformed into Java code, is configured as required for each pull request in the up-spec repo. However, this job is configured to only ever run, if some file(s) in the up-core-api folder are changed as well.

sophokles73 commented 4 months ago

The way I see it, we have two options

  1. configure the job to always run for every PR on up-spec, or
  2. do not make the job mandatory for PRs to be merged.

Personally, I would prefer option 2 in order to reduce the runtime of CI checks/build for PRs that do not change proto3 files.

@stevenhartley WDYT?

stevenhartley commented 4 months ago

@sophokles73 I suggest a 3rd option which is to configure the job to only run when there are changes to the up-core-api folder.

sophokles73 commented 4 months ago

That is already the case, and that is also the problem. The job does not run when there are no changes to the up-core-api folder. But GitHub prevents merging unless the output of this job is available ....

stevenhartley commented 4 months ago

OK I know then where the bug is, it is in the branch protection rules of .eclipsefn?

sophokles73 commented 4 months ago

Yes, that's option 2 ...

stevenhartley commented 4 months ago

OK found the bug, here is fix: https://github.com/eclipse-uprotocol/.eclipsefdn/pull/51

stevenhartley commented 4 months ago

Fixed by https://github.com/eclipse-uprotocol/.eclipsefdn/pull/51