#982 Architects are commanders for all commands now #1025
Conversation
|
@original-brownbear Thanks, I will find someone to review this PR soon |
|
@pinaf review this plz |
| ) | ||
| ), | ||
| String.format( | ||
| "/p/entry[@key='%s']/entry[@key='commanders']/item/text()", |
There was a problem hiding this comment.
@pinaf I wanted to reproduce the currently broken case where someone is an architect but not set as a commander for the deploy command.
In this case com.rultor.agents.Agents#commanders would instantiate QnAskedBy with exactly that xpath and should still see the architect as a commander for the deploy command,
even though that part of the .yml ( and by extension that xpath ) does not contain the architects login.
Hope that explanation makes sense :)
There was a problem hiding this comment.
@original-brownbear ok and the test fails without the change in QnAskedBy, correct?
There was a problem hiding this comment.
@pinaf jup :) without the change you only get the commanders from :
logins.addAll(new Crew(repo).names());
logins.addAll(xml.xpath(this.xpath));
and hence the auth check fails because the user is in neither list.
It's the exact situation my account here faces right now reproduced:
I'm not to be found under the above xpath, neither am I a collaborator on the repo => test fails.
|
@rultor merge |
|
@pinaf thanks! :) |
|
@rultor deploy |
@original-brownbear Sorry, I accept such requests only from authorized commanders: @yegor256, @alex-palevsky, @ |
@original-brownbear I'm sorry, I don't understand you :( Check this page and try again please |
|
@original-brownbear looks like it didn't work. |
#982 is resolved by this.