Skip to content

Conversation

@JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented Jan 24, 2022

049244a

[EWS] Support pull requests in Trigger
https://bugs.webkit.org/show_bug.cgi?id=235545
<rdar://problem/87992990 >

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(Trigger.__init__): Pass flags for patch and pull requests.
(Trigger.propertiesToPassToTriggers): Pass different properties if Triggered by a
pull request verse a patch.
(CompileWebKit.evaluateCommand): Pass flags for patch and pull request to Trigger.
(AnalyzeLayoutTestsResults.retry_build): Ditto.


Canonical link: https://commits.webkit.org/246384@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288552 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Copy link
Member

@aj062 aj062 left a comment

Choose a reason for hiding this comment

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

r+ with few comments.

Copy link
Member

Choose a reason for hiding this comment

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

This seems weird to have github.base.sha here. We should assign this only for patch workflow for now. This can create confusion later-on.

I believe we aren't using ews_revision anywhere in PR work-flow. If we need to use it later-on in PR workflow, let's add it in the same PR which will use it. Also we are already passing github.base.sha directly (in this PR), so we don't need another variable ews_revision for the same thing in PR workflow.

@JonWBedard JonWBedard force-pushed the eng/ews-github-trigger branch from 65dee98 to 3f0d13e Compare January 25, 2022 16:40
@JonWBedard JonWBedard closed this Jan 25, 2022
@JonWBedard JonWBedard force-pushed the eng/ews-github-trigger branch from 3f0d13e to 049244a Compare January 25, 2022 16:47
@JonWBedard JonWBedard merged commit 049244a into WebKit:main Jan 25, 2022
@JonWBedard
Copy link
Member Author

@JonWBedard JonWBedard deleted the eng/ews-github-trigger branch January 25, 2022 16:48
@aj062
Copy link
Member

aj062 commented Jan 25, 2022

Landed https://commits.webkit.org/246384@main (049244a)!

@JonWBedard include_revision portion is incorrect in this commit. It has been totally removed and 'ews_revision ' is not being passed at all. patch workflow still needs ews_revision, and it still needs to use include_revision appropriately. This part should have remained unchanged for patch workflow. Can you please fix it in a follow-up commit.

@JonWBedard
Copy link
Member Author

Landed https://commits.webkit.org/246384@main (049244a)!

@JonWBedard include_revision portion is incorrect in this commit. It has been totally removed and 'ews_revision ' is not being passed at all. patch workflow still needs ews_revision, and it still needs to use include_revision appropriately. This part should have remained unchanged for patch workflow. Can you please fix it in a follow-up commit.

Oh, oops! Yeah, that's going to be a problem. Will get it fixed ASAP.

@aj062
Copy link
Member

aj062 commented Jan 26, 2022

Oh, oops! Yeah, that's going to be a problem. Will get it fixed ASAP.

@JonWBedard Follow-up fix in https://trac.webkit.org/r288558 also has a mistake, it is setting the ews_revision property for pull_request workflow, while it should set it for patch workflow instead.

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