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

EascyCLA doesn't check for co-authors #3884

Open
molssongroup opened this issue Apr 3, 2023 · 40 comments
Open

EascyCLA doesn't check for co-authors #3884

molssongroup opened this issue Apr 3, 2023 · 40 comments
Assignees
Labels
01 - High High Priority bug Something isn't working

Comments

@molssongroup
Copy link
Member

Ticket: SUPPORT-17284

The reporting user wants to know if EasyCLA checks for co-authors. They state, "...I make tests (first, second, third) and EasyCLA ignore Co-authors, but check authors, no matter is the PR diff include their work or not."

Does EasyCLA not check for co-authors?

@molssongroup molssongroup added the bug Something isn't working label Apr 3, 2023
@dealako
Copy link
Member

dealako commented Apr 3, 2023

EasyCLA checks each and every commit author associated with a given comment. EasyCLA also checks all the commits associated with the pull request/merge request.

Can you provide a specific example of this not working?

@mlehotskylf
Copy link
Contributor

@WillsonHG
Copy link
Member

We need to test this to see if 1) are the logs reporting the co-author information and are we checking for this authorization and then 2) if so, this may be a display issue. @nickmango @thakurveerendras

@thakurveerendras
Copy link
Contributor

Hello @nickmango , @umeshlumbhani247
Kindly refer steps to replicate this issue

  1. Clone repo on local system
  • Add git branch : git checkout -b coauth14
  1. Made some changes on file
  • git add .
  • git commit -m "Add commit by auth" --author "thakurveerendras1 veerendrat+1@proximabiz.com" --signoff
  • git push origin coauth14
  1. Made some changes on file again
  • git add .
  • git commit -m "test Co-auth" -m "Co-authored-by: vrthakur thakur.viren17@gmail.com"
  • git push origin coauth14
  1. Login GitHub via user thakurveerendras1
  2. create PR https://github.com/sun-test-org/repo1/pull/31/commits
    image

@wanyaland
Copy link
Contributor

@molssongroup currently EasyCLA does not receive co-author information from github api and we are working on a workaround for this issue. Thanks

@nickmango nickmango assigned nickmango and unassigned wanyaland Apr 12, 2023
nickmango added a commit to nickmango/easycla that referenced this issue Apr 12, 2023
- Handled use case for getting co-author info in a given commit

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit to nickmango/easycla that referenced this issue Apr 12, 2023
- Handled use case for getting co-author info in a given commit

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit to nickmango/easycla that referenced this issue Apr 12, 2023
- Handled use case for getting co-author info in a given commit

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Apr 12, 2023
nickmango added a commit to nickmango/easycla that referenced this issue Apr 12, 2023
- Resolved fetching commit co-authors

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Apr 12, 2023
@thakurveerendras
Copy link
Contributor

user is missing the User's ID appears for co-authors, kindly refer below screenshot
PR : sun-test-org/repo1#33
image

nickmango added a commit to nickmango/easycla that referenced this issue Apr 12, 2023
- Added co-author user fetch by email

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Apr 12, 2023
@thakurveerendras
Copy link
Contributor

Reported issue is not reproducible , refer PR details

PR: sun-test-org/repo1#34
image

@thakurveerendras
Copy link
Contributor

commit added by main author details missing , refer steps to replicate it

  1. Clone repo on local system via user thakurveerendras
  2. Add git branch : git checkout -b coauth14
  3. Made some changes on file
  4. git add .
  5. git commit -m "Add commit by auth" --author "thakurveerendras1 veerendrat+1@proximabiz.com" --signoff
  6. git push origin coauth14
  7. Login GitHub via user thakurveerendras1
  8. create PR : Coauth18 sun-test-org/repo1#35

image

@wanyaland
Copy link
Contributor

@thakurveerendras for testing use this format to add co-authors
$ git commit -m "Refactor usability tests.

Co-authored-by: NAME NAME@EXAMPLE.COM

nickmango added a commit to nickmango/easycla that referenced this issue Apr 13, 2023
- Added co-committer in Github PR checks when gating

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Apr 13, 2023
@thakurveerendras
Copy link
Contributor

Now unknow "@web-flow " added on easycla boat , @nickmango kindly check

image

@mlehotskylf
Copy link
Contributor

@thakurveerendras to re-test.

@thakurveerendras
Copy link
Contributor

Tested issue on dev site & found that it is fixed as expected

image

@thakurveerendras
Copy link
Contributor

Tested issue on production & found that it is deployed as expected, So making issue status QA-Done

image

@jarias-lfx
Copy link

Affected PR finos/FDC3-conformance-framework#174

@jarias-lfx
Copy link

Received report for another PR with the same issue finos/FDC3#890

@jarias-lfx
Copy link

Affected PR eslint/eslint#17132

nickmango added a commit to nickmango/easycla that referenced this issue Jun 7, 2023
- Handled failed exceptions when getting co-author details resolving missingID
- Refactored search using email

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Jun 8, 2023
@jarias-lfx
Copy link

Adding another PR with co-author issue: open-telemetry/opentelemetry-demo#933
Where the author being fetch in EasyCLA is not the author in the commit.
Please check logs @nickmango

nickmango added a commit to nickmango/easycla that referenced this issue Jun 9, 2023
- Handle github search by email with an empty list

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue Jun 9, 2023
@jarias-lfx jarias-lfx reopened this Feb 22, 2024
@jarias-lfx jarias-lfx added the 01 - High High Priority label Feb 23, 2024
@mlehotskylf
Copy link
Contributor

We're reopening this ticket because we temporarily disabled this feature due to a potential performance issue. We're actively working to restore it as soon as possible.

@nickmango
Copy link
Collaborator

@thakurveerendras @umeshlumbhani247 re-activated this. Kindly review

@thakurveerendras
Copy link
Contributor

Tested on the dev site & found that coauthor detail is properly appears on PR

  • Refer to the screenshot

image
image

@umeshlumbhani247
Copy link
Collaborator

Hi, @mlehotskylf
I have finish testing on local and Dev stage, On Dev stage, After including co-authors, I got variation of 3% to 5% in Dev while comparing with previous benchmarks.
But in prod, depending on infrastructure, count may vary and variation can be less then Dev.
CC: @thakurveerendras @dealako

@thakurveerendras
Copy link
Contributor

Tested on prod & found that coauthor details are properly appears on cla-commit
image
image
image

@jarias-lfx
Copy link

@umeshlumbhani247 can you check kubernetes/website#45174 this one has co-authors and after running /easycla there are no updates. We don't have records for https://github.com/yermulnik and https://github.com/sh1895.
cc @mlehotskylf

@umeshlumbhani247
Copy link
Collaborator

@jarias-lfx
It seems a strange , The PR doesn't have a single easycla check comment earlier too.
Let me dig deep into this

@mlehotskylf
Copy link
Contributor

@nickmango identified the root cause and will deploy the fix to DEV later this week for QA testing.

nickmango added a commit to nickmango/easycla that referenced this issue May 1, 2024
- Updated aggregated co-authors fetched in the processing list

Signed-off-by: Harold Wanyama <hwanyama@contractor.linuxfoundation.org>
nickmango added a commit that referenced this issue May 1, 2024
@mlehotskylf
Copy link
Contributor

mlehotskylf commented May 2, 2024

@thakurveerendras can you please create 6 PRs to cover our use cases for co-author and send me the link:

  1. PR 1: [FAIL] PR with 1 co-author. Author and co-author should not sign EasyCLA.
  2. PR 2: [FAIL] PR with 1 co-author. Author should have signed EasyCLA, co-author should not have public email (missing ID case).
  3. PR 3: [FAIL] PR with 1 co-author. Author should have signed CLA, co-author should not.
  4. PR 4: [FAIL] PR with 1 co-author. Co-author should have signed CLA, author should not.
  5. PR 5: [PASS] PR with 1 co-author. Both author and co-author should have signed CLA.
  6. PR 6: [FAIL] PR with 2 co-author. Author and first co-author should have signed CLA, 2nd co-author should not.
  7. PR 7: [FAIL] PR with 2 co-author. First and second co-author should have signed CLA, author should not.
  8. PR 8: [PASS] PR with 2 co-author. Author and and both co-authors should have signed CLA.

@nickmango
Copy link
Collaborator

This is another usecase kubernetes/website#45174

@thakurveerendras
Copy link
Contributor

Hello @mlehotskylf , @nickmango
Kindly refer requested PR with there easycla & testing results

Case Summary Easycla status status PR link
1 PR with 1 co-author. Author and co-author should not sign EasyCLA. Not covered Passed sun-test-org/repo1#172
2 PR with 1 co-author. Author should have signed EasyCLA, co-author should not have public email (missing ID case) Not covered - missing ID Passed sun-test-org/repo1#169
3 PR with 1 co-author. Author should have signed CLA, co-author should not.} Not covered Passed sun-test-org/repo1#171
4 PR with 1 co-author. Co-author should have signed CLA, author should not. Not covered Passed sun-test-org/repo1#173
5 PR with 1 co-author. Both author and co-author should have signed CLA. Covered Passed sun-test-org/repo1#176
6 PR with 2 co-author. Author and first co-author should have signed CLA, 2nd co-author should not. Not covered Passed sun-test-org/repo1#174
7 PR with 2 co-author. First and second co-author should have signed CLA, author should not. Not covered Passed sun-test-org/repo1#175
8 PR with 2 co-author. Author and and both co-authors should have signed CLA. Covered Passed sun-test-org/repo1#177

@mlehotskylf
Copy link
Contributor

We are waiting for EasyCLA documentation to be updated before pushing this to PROD.

@mlehotskylf
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - High High Priority bug Something isn't working
Projects
None yet
Development

No branches or pull requests