Conversation
dainemawer
added
Infrastructure
jjgrainger
left a comment
jjgrainger
left a comment
There was a problem hiding this comment.
Looks good @dainemawer , just a couple of minor things to address.
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@dainemawer Left a couple of comments. This looks on the right track, but there's a lot of code here that can be simplified, and overall we need to be more careful of not enqueue things too aggressively.
…p conditional logic flow
|
@dainemawer FYI Since we've branched off from |
|
Roger that @felixarntz will fix the merge conflict here! |
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@dainemawer Left a bit more feedback here, this is looking close now.
Co-authored-by: Felix Arntz <[email protected]>
Make sure the function is only added when need it.
Co-authored-by: Felix Arntz <[email protected]>
Co-authored-by: Felix Arntz <[email protected]>
…ormance into feature/add-admin-pointer
Co-authored-by: Felix Arntz <[email protected]>
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
Awesome work @dainemawer!
Thanks @mitogh for the final iteration.
adamsilverstein
left a comment
adamsilverstein
left a comment
There was a problem hiding this comment.
one tiny spacing change to align the values, otherwise 👍🏼
Co-authored-by: Adam Silverstein <[email protected]>
5 participants
Summary
Fixes #193
Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.