Unable to merge PRs from external contributors

SAP / spartacus

Spartacus is a lean, Angular-based JavaScript storefront for SAP Commerce Cloud that communicates exclusively through the Commerce REST API.

Apache License 2.0

744 stars 389 forks source link

Unable to merge PRs from external contributors #4386

Open hackergil opened 5 years ago

hackergil commented 5 years ago

We need to modify our CI so that we can accept & merge PRs from external contributors

Example PR that can't be merged:

https://github.com/SAP/cloud-commerce-spartacus-storefront/pull/3766

marlass commented 5 years ago

Problems:

Jenkins doesn't run for forks (that should be fixed with migration to modelT and Travis for e2e tests
sonarcloud only works for internal pull requests (https://docs.travis-ci.com/user/sonarcloud/#analysis-of-internal-pull-requests)

Solution:

migrate from Jenkins (dependency on #3980)
remove sonarcloud for mandatory checks, but keep it as an optional (if that will fail Travis, improve script so it will not throw)

Xymmer commented 5 years ago

dependency on jenkins migration to travis, and we';ll continue discussion with gil on sonarcloud issue

Xymmer commented 5 years ago

marcin to ask tribe if everyone is seeing no sonarcloud issues

hackergil commented 4 years ago

Both issues exist. Prioritizing this work now.

hackergil commented 4 years ago

A workaround for this is that the reviewer (from inside SAP) to create a branch based on the PR (or fork). That would trigger both the sonar scan and you could also trigger a Jenkins build

marlass commented 4 years ago

Jenkins issue should be fixed by migrating with e2e test to Travis/github. For solar we might consider making it optional for merge. Let's keep this ticket.

hackergil commented 4 years ago

Blocked until we can run e2es in public CI

marlass commented 4 years ago

We can come back to this issue, as we now have a lot of backend instances on Azure.

marlass commented 4 years ago

Things to do:

skip --record --key $CYPRESS_KEY params when running cypress on forks (it currently fails)
use production build of the app instead of dev server (we already builded the app)