Date
1 - 5 of 5
[Zephyr-tsc] Github CODEOWNERS auto-assignment doesn't work as expected
alpi@...
Thanks Paul,
toggle quoted messageShow quoted text
Do you have some indication of when did this break? BR Alberto
-----Original Message-----
From: tsc@lists.zephyrproject.org [mailto:tsc@lists.zephyrproject.org] On Behalf Of Paul Sokolovsky via Lists.Zephyrproject.Org Sent: Friday 26 April 2019 11:10 To: devel@lists.zephyrproject.org; tsc@lists.zephyrproject.org Cc: tsc@lists.zephyrproject.org Subject: [Zephyr-tsc] Github CODEOWNERS auto-assignment doesn't work as expected Hello, As was noticed in a few (multiple?) PRs, automatic reviewer assignment based on the CODEOWNERS file no longer works as expected, specifically fails to assign reviewers. A recent example of this is https://github.com/zephyrproject-rtos/zephyr/pull/15670 - while there's an entry: /subsys/net/ip/ @jukkar @tbursztyka @pfalcon in CODEOWNERS, I wasn't requested for review. Again, that's not the only case. For a clean-room test, I submitted https://github.com/zephyrproject-rtos/zephyr/pull/15683 and as can be seen, the reviewer list is empty. This is problematic, because there's a risk that under-reviewed changes will get into the codebase, causing regressions. So, raising to the awareness of the TSC. -- Best Regards, Paul Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
|
|
Paul Sokolovsky
On Fri, 26 Apr 2019 09:25:17 +0000
"Alberto Escolar Piedras (ALPI)" <ALPI@oticon.com> wrote: Thanks Paul,Well, I was on vacation for two weeks before Apr 15. It definitely worked before that. And I saw first problematic case last week, after returning from vacation. No idea if that might be related to https://github.com/zephyrproject-rtos/zephyr/commit/636a7af43e704e25af29b31d565399fc02fef408 - nothing wrong is seen here. Actually, fishing for that link, I see that a whole bunch of changes was made to the CODEOWNERS file just recently: https://github.com/zephyrproject-rtos/zephyr/commits/master/CODEOWNERS Just imagine that any of those changes could break syntax a bit or propagation of changes to it could glitch on Github side, leading to the effect we see. And that's not counting a possible coincidence of Github tighting up the syntax for that file...
-- Best Regards, Paul Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog
|
|
Marc Herbert
[duplicate message cause the web interface doesn't seem to support cross-posting]
I don't know about the github side, however I just found and fixed a number of issues in the Zephyr script that checks the CODEOWNERS file, please help review: https://github.com/zephyrproject-rtos/ci-tools/pull/54
|
|
Marc Herbert
Hi,
I wasn't requested for review. Again, that's not the only case. [...]While github's features should really work as expected to save confusion and everyone's time, I hope this "under-review" risk is very small because I would expect every reviewer to... review the list of other reviewers first thing and add anyone they think could be missing and could help. Not just other co-owners, far from it. Database-backed review tools like github avoid broadcasts. This can be an advantage compared to pure email reviews but it requires a reasonable "networking" effort; otherwise there's a risk of ending up with the other extreme: semi-private, not very open-source reviews. I would also expect developers to err on the "oversubscription" side a bit because withdrawing from a review takes one click whereas you can't add yourself to something you don't know exist. The above is just based on experience with code reviews in general and more specifically the acute problem of finding reviewers' time. Everyone loves code reviews, yet for some reason I rarely ever see time scheduled for them :-) Thanks in advance for correcting any of the generalities above that wouldn't apply to Zephyr. I see that a whole bunch of changes was made to the CODEOWNERS fileIt'd be nice from github to share the corresponding parsing code; think open source. Marc
|
|
Marc Herbert
On Fri, Apr 26, 2019 at 02:49 AM, Paul Sokolovsky wrote:
Actually, fishing for that link, I see that a whole bunch of changesGood guess. Anas found the bug (commas) and fixed it at https://github.com/zephyrproject-rtos/zephyr/commit/af3f81ef04bccd0b959 Curious how Anas debugged this because I just found this user who put a lot of effort: http://www.benjaminoakes.com/git/2018/08/10/Testing-changes-to-GitHub-CODEOWNERS/ 2018-08-31 Github's answer:
So apparently no "API" available, it would have been documented at https://help.github.com/en/articles/about-code-owners otherwise. No need to ask github either as the feedback is already "in the right hands". Marc
|
|