Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/run prs with readwrite token #2676

Conversation

davidjgonzalez
Copy link
Contributor

No description provided.

@kwin
Copy link
Contributor

kwin commented Aug 25, 2021

@davidjgonzalez Why did you have to revert #2654 in the first place? (14fac98). What is different in this PR?

@davidjgonzalez
Copy link
Contributor Author

Nothing is different - i merged #2654 to master, then updated my branch (thinking it would fix my build issue) - but it just stalled my branch's build out, so i reverted it from master since it didnt seem to be working.

@kwin
Copy link
Contributor

kwin commented Aug 25, 2021

@davidjgonzalez Where is the build status of your fork's branch build? If it doesn't cause regressions I would like to keep it a while in master to check why forked PRs are still not working....

Also forked repo PRs are not an issue AFAIK for people having write access to ACS AEM Commons (https://github.com/orgs/Adobe-Consulting-Services/people) but only for people outside the committers group!
We should check with again once we have a PR from outside the committers group!

@davidjgonzalez
Copy link
Contributor Author

This was the PR ..

#2667

We can merge it in again to master if we want; I just don't know that PR's based on it will ever finish collecting status. I wanted to try to cut 5.0.8 tonight (and merge/re-run build across the open PR's takes so long - wanted to get a jump on it while i was working -- since updating w/ latest while time-consuming, is usually mindless :)

WDYT? Merge to master for now - and admin merge PR's if we need?

@kwin
Copy link
Contributor

kwin commented Aug 25, 2021

The issue Error: Resource not accessible by integration in the step Publish Test Report seems orthogonal to this PR (it fails with and without it!) Let's track that separately....

kwin
kwin previously approved these changes Aug 25, 2021
@kwin kwin dismissed their stale review August 25, 2021 15:50

One issue detected

@davidjgonzalez
Copy link
Contributor Author

@kwin ah - i thought they might be related due to: ScaCap/action-surefire-report#31
It looked like your PR addressed the issue w/ permissions of PR's from forks (which appears to be the issue here).

Either way - this issue's change seems to cause any build that included it to get stuck with:

2021-08-25 at 11 50 AM

I assume this is not expected behavior?

@kwin
Copy link
Contributor

kwin commented Aug 25, 2021

The current yaml has one issue:

GH reports

Invalid workflow file
The workflow is not valid. .github/workflows/maven.yml (Line: 50, Col: 11): A mapping was not expected 

(https://github.com/Adobe-Consulting-Services/acs-aem-commons/actions/runs/1167170469)

@davidjgonzalez
Copy link
Contributor Author

davidjgonzalez commented Aug 25, 2021

ah - ok - that explains the stuck build status ( ah - in Actions tab .. of course)

@kwin kwin force-pushed the feature/run-prs-with-readwrite-token branch 2 times, most recently from 4cbf4ea to 47946c3 Compare August 25, 2021 16:21
debug event with toJSON function
fix yaml
use caching integrated in setup-java
@kwin kwin force-pushed the feature/run-prs-with-readwrite-token branch from 47946c3 to bb7b2ef Compare August 25, 2021 16:24
@kwin kwin merged commit 2769058 into Adobe-Consulting-Services:master Aug 25, 2021
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.

2 participants