- 
                Notifications
    
You must be signed in to change notification settings  - Fork 376
 
Fail early for the resource conflicts between the activating controllers #2760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fail early for the resource conflicts between the activating controllers #2760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty elegant, thank you!
          
 I don't get the failing tests, I'll take a look when I get off the plane. Thanks  | 
    
| 
           Sorry for the different committer naming, I did this from a colleagues laptop hahah  | 
    
          Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@            Coverage Diff             @@
##           master    #2760      +/-   ##
==========================================
- Coverage   89.62%   89.59%   -0.03%     
==========================================
  Files         152      152              
  Lines       17637    17710      +73     
  Branches     1448     1454       +6     
==========================================
+ Hits        15807    15868      +61     
- Misses       1246     1256      +10     
- Partials      584      586       +2     
 Flags with carried forward coverage won't be shown. Click here to find out more. 
 🚀 New features to boost your workflow:
  | 
    
| 
           Unfortunately, this does not fix #2758 for me. The output is the same as described in the issue.  | 
    
          
 Thanks for commenting about it. I'll recheck the tests and try to run UR tests locally.  | 
    
| 
           @urfeex With the new changes, your issues should be fixed. Please test it and let us know. I ran the UR tests locally and it passed. Please check and let us know tomorrow :)  | 
    
| 
           That fixes the regression and also looks fine :-) I'll approve with my private account which is in the org. Thank you very much for addressing this so quickly @saikishor  | 
    
          
 I'm glad it worked ;)  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes: #2758
@urfeex FYI