-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add configuration for CLA Assistant GitHub Action #15
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #15 +/- ##
=======================================
Coverage 90.19% 90.19%
=======================================
Files 7 7
Lines 306 306
=======================================
Hits 276 276
Misses 30 30 ☔ View full report in Codecov by Sentry. |
I've added the yml to a dumy repo to test and found a couple of things. Namely, the |
So, the bot does a full isequal check before accepting sign comments (meaning the comment has to be exactly equal to the value set in custom-pr-sign-comment). We therefore can not add additional info into the single sign comment. Other then that, everything works now in my mock test. |
thanks for testing! Did you check with the YML version from this PR? |
I tested with the yml from here. I think the code you modified only decides if the comment triggers a CLA-Assistant action, not what the action is going to be, AFAIK this implements the comparison to the reference comment. Anyways, here are my suggested changes.
|
Co-authored-by: Ronald Jäpel <[email protected]>
Thanks @ronald-jaepel I applied your changes and reversed my change in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
Co-authored-by: Martin Beyß <[email protected]>
.github/workflows/cla.yml
Outdated
|
||
\- [ ] The JuBiotech maintainers know my current employer. | ||
|
||
\- [ ] I am making not making this contribution on behalf of my current employer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt this be
I am not making this contribution on behalf of my current employer.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if the contribution is made in the contributors free time, their current employer is irrelevant. They might also be self- or unemployed. Therefore only one of the two task items must apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get that, I just think there is one making too many in the second point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\- [ ] I am making not making this contribution on behalf of my current employer. | |
\- [ ] I am *not* making this contribution on behalf of my current employer. | |
ah of course, sorry my in-mind-diffing was incomplete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we need to consult Alexander Storm here. In my view, it makes a difference if somehone has an employer of if he/she made the contribution during working hours. To make things more complex, contributions in "free time" might still require permission of thge employer (or not). This is not our business but it must be clear to the contributor which option to choose. Some contributors (like Sam) have multiple accounts to clarify on/off work contributions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, then I will summarize the proposed procedure, texts & mockup screenshots, and consult him by email (cc y'all).
Internal ticket: https://jugit.fz-juelich.de/IBG-1/rse/proxy-issues/-/issues/17