Don't die when finding an unknown email
ClosedPublic

Authored by aacid on Jan 27 2019, 7:35 PM.

Details

Summary

Nowadays with phabricator, etc. it's very common to have commits whose author is not a committer and thus not listed in the accounts file.

Instead of die-ing collect all those emails and print them in the summary, the script will just work fine

Diff Detail

Repository
R230 KDE Development Scripts
Branch
arcpatch-D18568
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7643
Build 7661: arc lint + arc unit
aacid requested review of this revision.Jan 27 2019, 7:35 PM
aacid created this revision.

Looks good to me. I had also been considering the possibility of an authors-accounts file (along with accounts and disabled-accounts) but I don't know that it would add anything to what's already proposed here, and maintaining that file would probably need to be done manually.
One problem remains, however. If such a listed author agrees to be added to the script - what should be used as username, since there isn't an account? Would simply adding the email address instead of username be reasonable?

This is the limit of my perl knowledge.

relicensecheck.pl
415

This is a map. shouldn't an array (@unknown_authors) be used here?

489

push (@unknown_authors, $email)

642

foreach my $who (@unknown_authors)

Being Perl, It could be done either way. The current code just repeats creating the hash item, which is essentially a no-op. If you use a list, you would have to check if the email is already on the list before pushing it onto the list. (You could always sort and remove dups before printing, but the hash seems simpler to me.

@svuorela As Jack says if i do that we get duplicates, this is the easy way of not getting duplicates, but it's also at the limit of my perl knowledge

@ostroffjh yes, if someone that isn't a commiter wants to be added to that file, it can be done by adding

'email' => 'email',

in secondary_mail_addresses and then

'email' => [licenses],

in license_table

The problem with that is how we actually verify that he indeed about relicensing since previously it was very easy, you only added yourself, so that was clear you agreed.

I guess one way would be for them to create a phabricator request.

So, if the person made the original commit by Phabricator, he can request his addition to the script the same way.
Separately, there sill is the case (don't know if real or only potential) of someone making a contribution by sending to a mailing list, so he might not actually have any KDE account at all. I suppose that in that case, if the person had any interest in KDE, it would be reasonably to request he get a basic account (identities.kde.org, I assume).
Finally, would there be any reasonable way to use that basic KDE accountname as a username? (Am I correct a dev account is just a basic KDE account which is in teh developers group? If so, we know there wouldn't be any overlap in names.) I don't think we want a file with every kde account listed, due to the size - might there be an "authors" group for such purpose? I'm just afraid that it wouldn't be easy to maintain. It would need anyone listed as a git author, but who is not already in the developer group.) For now, I'd say let's get the current proposal committed, then we can discuss the additional issues.

When using a hash as a set, there is little point in repeating the key into the value. I'd just do

$unknown_authors{$email} = 1;
aacid updated this revision to Diff 50460.Jan 28 2019, 11:30 PM
  • update according to dfaure's comment
dfaure accepted this revision.Jan 29 2019, 7:43 AM
This revision is now accepted and ready to land.Jan 29 2019, 7:43 AM
This revision was automatically updated to reflect the committed changes.