Extend mail characteristics2 #82
Conversation
Signed-off-by: Sebastian Duda <[email protected]>
Signed-off-by: Sebastian Duda <[email protected]>
| return False | ||
|
|
||
| def _integrated_correct(self, repo, maintainers_version): | ||
| if self.first_upstream in repo: | ||
| upstream = repo[self.first_upstream] | ||
| self.committer = upstream.committer.name.lower() |
There was a problem hiding this comment.
Wollen wir hier vielleicht die Mailadresse ausleiten? Die ist aussagekräftiger, oder?
There was a problem hiding this comment.
Okay, I remember that - for some reason - we talked about this change, but I don't understand at the moment why we did this. Could you please factor this change out to an own commit with an explanation why we do this? This change is preparatory work for supporting other projects, and not related to the introduction of Jailhouse.
There was a problem hiding this comment.
We need this to get the committer of patches in projects without a maintainer.
Without this change, we would return from this function before we get this information.
The information about the committer however is relevant and valid for all projects.
|
|
||
|
|
||
| class JailhouseMailCharacteristics(MailCharacteristics): | ||
| ROOT_DIRS = ['ci', |
There was a problem hiding this comment.
Hier müsstest du mir noch das Skript zukommen lassen, mit dem du alle Versionen überprüft hast.
Andernfalls scripte ich das selbst.
There was a problem hiding this comment.
Yep, just sent it to you. The directories seem complete!
| """ | ||
| PaStA - Patch Stack Analysis | ||
|
|
||
| Copyright (c) OTH Regensburg, 2021 |
There was a problem hiding this comment.
OTH is not the copyright holder in this case. So this is either your university, or you.
|
|
||
|
|
||
| class JailhouseMailCharacteristics(MailCharacteristics): | ||
| ROOT_DIRS = ['ci', |
There was a problem hiding this comment.
Yep, just sent it to you. The directories seem complete!
| 'setup.py', | ||
| 'TODO.md', | ||
| '.travis.yml', | ||
| 'VERSION', |
There was a problem hiding this comment.
We miss:
driver.c
jailhouse.h
README (there was an ancient version without .md)
TODO (same here)
Rest looks good!
| return False | ||
|
|
||
| def _integrated_correct(self, repo, maintainers_version): | ||
| if self.first_upstream in repo: | ||
| upstream = repo[self.first_upstream] | ||
| self.committer = upstream.committer.name.lower() |
There was a problem hiding this comment.
Okay, I remember that - for some reason - we talked about this change, but I don't understand at the moment why we did this. Could you please factor this change out to an own commit with an explanation why we do this? This change is preparatory work for supporting other projects, and not related to the introduction of Jailhouse.
| @@ -197,8 +204,6 @@ def check_maintainer(section, committer): | |||
| # integrated by a maintainer that is responsible for a section that is | |||
| # affected by the patch. IOW: The field indicates if the patch was | |||
| # picked by the "correct" maintainer | |||
| upstream = repo[self.first_upstream] | |||
| self.committer = upstream.committer.name.lower() | |||
There was a problem hiding this comment.
And what about the above comments? They now point into the void?
| # maintainers analysis. | ||
| patches = ids - repo.mbox.invalid | ||
| tags = {repo.patch_get_version(repo[x]) for x in patches} | ||
| maintainers_version = load_maintainers(config, tags) |
There was a problem hiding this comment.
What about all other projects that do not have HAS_MAINTAINERS yet? They are now broken with this change. Please factor this change out to a preparatory patch, then we get a better overview how we can handle this.
| from .LinuxMailCharacteristics import LinuxMailCharacteristics | ||
| from .QemuMailCharacteristics import QemuMailCharacteristics | ||
| from .UBootMailCharacteristics import UBootMailCharacteristics | ||
| from .XenMailCharacteristics import XenMailCharacteristics | ||
| _load_characteristics = { | ||
| 'jailhouse': (load_maintainers_characteristics, JailhouseMailCharacteristics), |
There was a problem hiding this comment.
And if we once have factored out those things, we might even see a better way to handle this dictionary here.
| @@ -31,6 +31,7 @@ | |||
| _tmp_repo = None | |||
|
|
|||
| mainline_regex = { | |||
| 'jailhouse': re.compile(r'^v.*$'), | |||
| @@ -76,7 +76,7 @@ class MailCharacteristics: | |||
| BOTS = {'[email protected]', '[email protected]', | |||
| '[email protected]', '[email protected]'} | |||
| POTENTIAL_BOTS = {'[email protected]', '[email protected]'} | |||
| PROCESSES = ['linux-next', 'git pull', 'rfc'] | |||
| PROCESSES = ['linux-next', 'git pull', 'rfc', '[PULL]'] | |||
There was a problem hiding this comment.
Yeah, sounds valid. But what about having everything here in lower case, and comparing it against lower case? With this, we would also catch [pull].
| return True | ||
|
|
||
| # Buildroot's daily results bot | ||
| if '[autobuild.buildroot.net] Daily results' in subject: |
There was a problem hiding this comment.
Please compare against subject.lower()
There was a problem hiding this comment.
this is not necessary since subject = email_get_header_normalised(self.message, 'subject') already lowers the subject.
There was a problem hiding this comment.
Oh, you're right - but then 'Daily results' will never match.
| return True | ||
|
|
||
| # Buildroot's daily results bot | ||
| if 'oe-core cve metrics' in subject.lower(): |
There was a problem hiding this comment.
And then you can combine those two cases.
There was a problem hiding this comment.
And the .lower() is redundant here.
rralf
force-pushed
the
next
branch
2 times, most recently
from
56b12df
to
96f2fd9
Compare
This PR adds Jailhouse-support for the ignored patch analysis