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

oca-cla-bot fails with old commits and old emails #209

Open
yajo opened this issue Mar 22, 2022 · 8 comments
Open

oca-cla-bot fails with old commits and old emails #209

yajo opened this issue Mar 22, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@yajo
Copy link
Member

yajo commented Mar 22, 2022

Describe the bug

oca-cla-bot fails when somebody migrates a module that includes commits from an email that had CLA signed back then, but not anymore.

To Reproduce

See:

Expected behavior
My old job email should still pass ✔️ because when I pushed those commits back then, I had the CLA signed with that email.

@yajo yajo added the bug Something isn't working label Mar 22, 2022
@rousseldenis
Copy link

@yajo I think this is tied to how we do migrations.

IMHO (I'll try to open a discussion soon), migrations should include only migration commits. It's becoming more and more difficult to review migrations.

We should create new main branches from preceding one.

@pedrobaeza
Copy link
Member

That procedure was a total pain, as there can be extra commits since the branch creation, and doing a periodical synchronization is also very tedious, provoking at the end also non pure migration PRs.

But you can review a PR with several commits in the current procedure, including the migration commit, very easily clicking on such commit, and GitHub shows the diff related to the migration. What is the problem with it? What is becoming more and more difficult? This procedure also serves for squashing useless administrative commits.

@yajo
Copy link
Member Author

yajo commented Mar 24, 2022

Well, it truly has a security problem, but outside of that I agree that the old process was even worse.

In any case, all of that is outside of the scope. The problem is that the bot complains about email addresses that were CLA-signed back then. It should compare the email and date of the commit, and then complain about those that have no CLA approval within that specific date.

@pedrobaeza
Copy link
Member

Yes, I also put the same on #170. @sbidoul @gurneyalex maybe you can check it.

@sbidoul
Copy link
Member

sbidoul commented Apr 3, 2022

I don't think there is anything reasonable we can do to fix the current CLA bot. Long term we may want to move to a paperless and more robust process with, e.g., https://github.com/cla-assistant/cla-assistant.

@pedrobaeza
Copy link
Member

@sbidoul we just need to include in the bot with_context(active_test=False) for searching inactive contacts

@gurneyalex
Copy link
Member

I just added the context key in the legacy clabot. Let's see if it helps.

@SirTakobi
Copy link
Contributor

Has #209 (comment) fixed the issue? Can this be closed?

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

No branches or pull requests

6 participants