Skip to content

Arms & Events PR fixes#1

Merged
patking02 merged 5 commits into
patking02:masterfrom
redcap-tools:arms-events-pr-fixes
Oct 31, 2023
Merged

Arms & Events PR fixes#1
patking02 merged 5 commits into
patking02:masterfrom
redcap-tools:arms-events-pr-fixes

Conversation

@pwildenhain
Copy link
Copy Markdown

With these fixes, the integration tests and doctests will pass. I'm happy to answer any questions you have about the proposed changes. Feel free to change/amend syntax to whatever suites you best

Copy link
Copy Markdown
Owner

@patking02 patking02 left a comment

Choose a reason for hiding this comment

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

These look great; thanks for the review.

@pwildenhain
Copy link
Copy Markdown
Author

pwildenhain commented Oct 31, 2023

You're welcome! Can you merge this PR into your branch? I don't have permissions to do that. Don't mind the CI failures, they should be fixed when this gets merged into master

@patking02
Copy link
Copy Markdown
Owner

patking02 commented Oct 31, 2023

Yes. And I think in the ci.yml file, on line 40 if you swap out:
if: github.actor == 'pwildenhain'

With:
if: github.triggering_actor == 'pwildenhain'

I think then doctest's will not get run in a fork. It thinks you were the initial github 'actor' since there was some sort of Build 1 when the PR was created here, and then when I tried Build 2 I think it failed (I think the build keeps your username as the actor, but maybe triggering_actor would work...)

https://docs.github.com/en/actions/learn-github-actions/contexts#github-context

@patking02 patking02 merged commit 46bbf78 into patking02:master Oct 31, 2023
@pwildenhain pwildenhain deleted the arms-events-pr-fixes branch October 31, 2023 18:21
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