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

Run write-manifest on PRs to dev #773

Merged
merged 45 commits into from
May 7, 2024
Merged

Conversation

jthompson-arcus
Copy link
Collaborator

Addresses #772

Copy link

codecov bot commented May 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.61%. Comparing base (4fa6e61) to head (42bf8be).
Report is 50 commits behind head on dev.

❗ Current head 42bf8be differs from pull request most recent head 8881159. Consider uploading reports for the commit 8881159 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #773   +/-   ##
=======================================
  Coverage   81.61%   81.61%           
=======================================
  Files          34       34           
  Lines        5277     5277           
=======================================
  Hits         4307     4307           
  Misses        970      970           

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

Copy link
Collaborator

@Jeff-Thompson12 Jeff-Thompson12 left a comment

Choose a reason for hiding this comment

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

Testing comment review

@Jeff-Thompson12 Jeff-Thompson12 self-requested a review May 3, 2024 15:36
Jeff-Thompson12
Jeff-Thompson12 previously approved these changes May 3, 2024
Copy link
Collaborator

@Jeff-Thompson12 Jeff-Thompson12 left a comment

Choose a reason for hiding this comment

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

Testing approval review

Jeff-Thompson12
Jeff-Thompson12 previously approved these changes May 3, 2024
Copy link
Collaborator

@Jeff-Thompson12 Jeff-Thompson12 left a comment

Choose a reason for hiding this comment

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

Approval testing 2

Jeff-Thompson12
Jeff-Thompson12 previously approved these changes May 3, 2024
Copy link
Collaborator

@Jeff-Thompson12 Jeff-Thompson12 left a comment

Choose a reason for hiding this comment

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

Testing approval 3

Jeff-Thompson12
Jeff-Thompson12 previously approved these changes May 3, 2024
Copy link
Collaborator

@Jeff-Thompson12 Jeff-Thompson12 left a comment

Choose a reason for hiding this comment

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

Approval testing 4

@jthompson-arcus
Copy link
Collaborator Author

@aclark02-arcus For some reason the checksum always updated for .Rprofile, so I decided to remove it from the manifest creation process. The main issue with this process is that the write-manifest is triggered after a reviewer approves the PR. The process then adds a commit which makes the review stale, requiring the reviewer to approve the request a second time as well.

Thoughts?

@jthompson-arcus jthompson-arcus requested review from Jeff-Thompson12 and aclark02-arcus and removed request for Jeff-Thompson12 and aclark02-arcus May 7, 2024 13:47
@jthompson-arcus
Copy link
Collaborator Author

Skipped in draft
image

@jthompson-arcus jthompson-arcus marked this pull request as ready for review May 7, 2024 14:09
@jthompson-arcus jthompson-arcus marked this pull request as draft May 7, 2024 14:11
@jthompson-arcus jthompson-arcus marked this pull request as ready for review May 7, 2024 14:11
@jthompson-arcus jthompson-arcus marked this pull request as draft May 7, 2024 14:14
@jthompson-arcus jthompson-arcus marked this pull request as ready for review May 7, 2024 14:24
@jthompson-arcus jthompson-arcus requested review from Jeff-Thompson12 and removed request for Jeff-Thompson12 May 7, 2024 14:24
@jthompson-arcus
Copy link
Collaborator Author

@aclark02-arcus I believe this has all the functionality we want now. Only negative I can think of now is that our normal checks are no longer sitting at the head of the branch.

Copy link
Collaborator

@aclark02-arcus aclark02-arcus left a comment

Choose a reason for hiding this comment

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

Amazing work!

@aclark02-arcus aclark02-arcus merged commit c548a09 into dev May 7, 2024
1 check passed
@jthompson-arcus jthompson-arcus deleted the jt-772-gha_for_manifest2 branch May 7, 2024 14:37
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.

4 participants