rultor
rultor copied to clipboard
yegor256
Rultor should not ask architect to confirm merge on a closed PR
See the following PR on the xockets-layer project:
In this project, I'm the architect and I mistakenly asked Rultor to merge when I meant deploy. It correctly told me that the PR is closed. However, a few minutes later the code reviewer also asked Rultor to merge (perhaps also by mistake), but it then asked me to confirm the merge.
This is confusing. If a PR is closed, Rultor should show the same message as the one it addressed to me, instead of asking me to confirm a command which would be rejected anyway..
@carlosmiranda I am aware of the task, give me some time to find a developer...
@carlosmiranda thanks for the ticket, your account was topped for 15 mins, payment 62918784
@dskalenko could you please pick this up? This article explains how we work. Any technical questions you may ask right here; The budget here is 30 mins, which is exactly how much time will be paid for, when the task is completed
@carlosmiranda @alex-palevsky I did some investigation and seems this situation, when Rultor may ask architect to confirm a command which would be rejected anyway, potentially may be not only for merge. So in this case fix should be done for all such commands (unlock, lock, merge, deploy, release ...). But the role of "Architect" can be violated in this case, because each command should be confirmed by architect at the beginning (at least I understood it from source). Example (described in this issue) really a little bit confused, but it's just because of commands done by mistake (I don't know if it happens often). And let's consider contra-example : UserA ask for merge closed PR-> Rultor ask for architect confirmation. In meantime owner of PR reopened particular PR. When architect confirm this command it would not be rejected. So from my opinion we have 3 options :
- leave as it is
- fix for all such commands
- try fix only for merge (for me it's a little bit complicated for current implementation) Hope you understand what I mean.
Please help me dispel my doubts and choose correct way. Thanks.
P.S. it's only my opinion - correct me if I am wrong.
@dpisarenko The situation I'm asking above specifically pertains to merge, so let's limit it to that. It's only impossible to re-merge a merged branch, deploy or release commands are not restricted.
@dskalenko this ticket has been with you quite a while now, any update ?
@original-brownbear I did fix of this issue, but since my fix was rejected and I have to create new test (which not so easy to do), I need more time.
@dskalenko no problem, but please when it comes to more time @alex-palevsky needs to be informed asap. I'm just here for the tech not the schedule. Just tell him you nee more time!
@dskalenko thanks for reporting that, could you give some time frame to expect here too please?
@alex-palevsky I need more time to complete this issue
@dskalenko no problem, thanks for letting me know
@dskalenko thanks for reporting that, could you give some time frame to expect here too please?
@original-brownbear I'll do my best to complete to the end of week
@alex-palevsky Please find other person to fix this issue. Thanks
@dskalenko sure, no problem
@alex-palevsky this is postponed.
@original-brownbear got it, "postponed" label here
@alex-palevsky this is postponed.
@original-brownbear someone else will help in this task, no problem at all