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

fix(ci): make upload artifacts job edit the comment #2054

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dorimedini-starkware
Copy link
Collaborator

No description provided.

@dorimedini-starkware dorimedini-starkware self-assigned this Nov 14, 2024
@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link

Artifacts upload triggered. View details here

Copy link

Artifacts upload triggered. View details here

body: `Artifacts upload triggered. [View details here](${workflowLink})`
})
token: ${{ secrets.GITHUB_TOKEN }}
issue-number: ${{ process.env.PR_NUMBER }}
Copy link

Choose a reason for hiding this comment

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

The environment variable reference should be ${{ env.PR_NUMBER }} instead of ${{ process.env.PR_NUMBER }}. GitHub Actions workflow syntax uses env directly to access environment variables, while process.env is Node.js syntax that won't work in this context.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@@ -213,6 +213,7 @@ impl BlockContext {
}
}

// tmp
Copy link

Choose a reason for hiding this comment

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

This comment // tmp appears to be unrelated to the CI changes in this PR and was likely committed accidentally. Please remove it in a separate commit or PR.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.00%. Comparing base (e3165c4) to head (f250d76).
Report is 380 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2054       +/-   ##
===========================================
+ Coverage   40.10%   69.00%   +28.89%     
===========================================
  Files          26      103       +77     
  Lines        1895    13562    +11667     
  Branches     1895    13562    +11667     
===========================================
+ Hits          760     9358     +8598     
- Misses       1100     3804     +2704     
- Partials       35      400      +365     

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

Copy link
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @graphite-app[bot])

body: `Artifacts upload triggered. [View details here](${workflowLink})`
})
token: ${{ secrets.GITHUB_TOKEN }}
issue-number: ${{ process.env.PR_NUMBER }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link

Artifacts upload triggered. View details here

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