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

[Do Not Merge] Duplicate PR #91

Closed
wants to merge 2 commits into from
Closed

Conversation

1e9abhi1e10
Copy link
Contributor

duplicate PR of #78

@1e9abhi1e10
Copy link
Contributor Author

@oscarbenjamin In this PR the added benchmark doesn't running in CI.

@oscarbenjamin
Copy link
Contributor

@brocksam should we expect that new benchmarks added in a PR will run in CI for the PR?

I'm not sure exactly what this does:

- name: Setup access to branches
run: |
git submodule add https://github.com/sympy/sympy_benchmarks.git
git remote add upstream https://github.com/sympy/sympy.git
git fetch upstream master
git fetch upstream 1.12

My guess is that uses the sympy_benchmarks master branch rather than the PR branch.

@1e9abhi1e10 1e9abhi1e10 changed the title [Do Not Merged] Duplicate PR [Do Not Merge] Duplicate PR Jul 5, 2023
@brocksam
Copy link
Contributor

brocksam commented Jul 6, 2023

@brocksam should we expect that new benchmarks added in a PR will run in CI for the PR?

I agree this is the behaviour that want, but as evidenced above clearly isn't the case right now.

I'm not sure exactly what this does:

- name: Setup access to branches
run: |
git submodule add https://github.com/sympy/sympy_benchmarks.git
git remote add upstream https://github.com/sympy/sympy.git
git fetch upstream master
git fetch upstream 1.12

My guess is that uses the sympy_benchmarks master branch rather than the PR branch.

Agreed, I've opened PR #92 for this.

@1e9abhi1e10 1e9abhi1e10 closed this Jul 6, 2023
@1e9abhi1e10 1e9abhi1e10 deleted the duplicate branch July 6, 2023 19:06
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