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

Commit hash of PR HEAD not found fix #2747

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

officialasishkumar
Copy link
Member

📝 Description

Type: 🪲 bugfix

In the earlier changes that I made was running fine on my fork action runs but I only checked that by creating pull request from my own account (which did not involved creating a fork). The latest action in tardis fails to read the commits of the PR which i am suspecting to be the reason of a PR created from a fork. So, I tried to make the same environment by creating a fork of my tardis repo from a different account and then making a pull request from that account. The action ran on that PR which can be found at https://github.com/officialasishkumar/tardis/actions/runs/10113490866/job/27969978963. The commit hash are now getting detected by asv as you can see at "Run benchmarks for base and head commits of PR". I hope this fixes the issue.

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe)
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request
  • I updated the documentation according to my changes
  • I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

Signed-off-by: Asish Kumar <[email protected]>
@tardis-bot
Copy link
Contributor

tardis-bot commented Jul 26, 2024

*beep* *bop*
Hi human,
I ran ruff on the latest commit (88e5770).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Complete output(might be large):

fetch-depth: 0
if: github.event_name == 'pull_request_target'

- name: fetch master branch
Copy link
Contributor

Choose a reason for hiding this comment

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

Sentence case Fetch master branch

andrewfullard
andrewfullard previously approved these changes Jul 26, 2024
Signed-off-by: Asish Kumar <[email protected]>
@andrewfullard andrewfullard enabled auto-merge (squash) July 26, 2024 16:00
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.12%. Comparing base (051effd) to head (88e5770).
Report is 6 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2747       +/-   ##
===========================================
+ Coverage   36.50%   69.12%   +32.61%     
===========================================
  Files         186      188        +2     
  Lines       14922    14943       +21     
===========================================
+ Hits         5448    10330     +4882     
+ Misses       9474     4613     -4861     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tardis-bot
Copy link
Contributor

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (bdf2df3) and the latest commit (88e5770).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

· Did not find results for commit 88e577064c8cc8bfa3dd6779ac85e26e64fd3a54

All benchmarks:

· Did not find results for commit 88e577064c8cc8bfa3dd6779ac85e26e64fd3a54

If you want to see the graph of the results, you can check it here

@andrewfullard andrewfullard merged commit ced499e into tardis-sn:master Jul 26, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants