Skip to content

Conversation

@aelovikov-intel
Copy link
Contributor

If the PR's source branch is already inside origin then it is known to be safe.

If the PR's source branch is already inside `origin` then it is known to
be safe.
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner July 17, 2023 23:24
@aelovikov-intel
Copy link
Contributor Author

@aelovikov-intel aelovikov-intel requested a review from bader July 17, 2023 23:25
@bader
Copy link
Contributor

bader commented Jul 18, 2023

Tested in https://github.com/intel/llvm/pull/10441/checks.

You use sycl-devops-pr prefix for your test branch and we already allow running modified scripts for them. Right?

@aelovikov-intel
Copy link
Contributor Author

Tested in https://github.com/intel/llvm/pull/10441/checks.

You use sycl-devops-pr prefix for your test branch and we already allow running modified scripts for them. Right?

Yes, it's been described in PR/commit message for #10002.

@bader
Copy link
Contributor

bader commented Jul 18, 2023

Tested in https://github.com/intel/llvm/pull/10441/checks.

You use sycl-devops-pr prefix for your test branch and we already allow running modified scripts for them. Right?

Yes, it's been described in PR/commit message for #10002.

Right. I don't think https://github.com/intel/llvm/pull/10441/checks really checks this change.

@aelovikov-intel
Copy link
Contributor Author

Right. I don't think https://github.com/intel/llvm/pull/10441/checks really checks this change.

I think it does. Initially I've created the PR from a branch with the same name in my fork and its testing didn't start - see https://github.com/intel/llvm/pull/10440/checks.

@bader
Copy link
Contributor

bader commented Jul 18, 2023

Right. I don't think https://github.com/intel/llvm/pull/10441/checks really checks this change.

I think it does. Initially I've created the PR from a branch with the same name in my fork and its testing didn't start - see https://github.com/intel/llvm/pull/10440/checks.

But the goal of the change is to allow pre-commit for branches in intel/llvm, not for private forks. Isn't the goal of this change to enable pre-commit testing for LLVM pulldown PR? If so, we need to test it. IMO, negative testing is not enough and the use case from https://github.com/intel/llvm/pull/10441/checks should be already working.

@aelovikov-intel aelovikov-intel merged commit 0697e68 into sycl Jul 18, 2023
@aelovikov-intel aelovikov-intel deleted the sycl-devops-pr/allow-pulldown branch July 18, 2023 23:18
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.

3 participants