Repository navigation
Fix Amazon ECS Enums - #29871
Fix Amazon ECS Enums#29871
Conversation
boring-cyborg
Bot
added
area:providers
provider:amazon
|
I've just figure out that there is no tests for ECS Sensors 🙄 |
c958084 to
6274a9d
Compare
There was a problem hiding this comment.
Should we put this in a more general place so that other modules can make use of it?
There was a problem hiding this comment.
Right now it just for fix but in general we could move it later if it useful for other places, that why I make it "private"
There was a problem hiding this comment.
I was thinking the same thing. Seems like something we'll want to reuse.
There was a problem hiding this comment.
Agree. We might want to do it now? It'd encourage others to use if needed instead of copy pasting it
There was a problem hiding this comment.
I feel like it is quite trivial to move it to a more generic place (and still keep it private if you'd prefer that). Seems easy to make the right choice now rather than having to fix it later. But I wont press any harder, happy to merge this as is and I can fix it in a follow up PR 👍
There was a problem hiding this comment.
I'd rather move it in separate PR when we actually need this functional. Just for avoid spending time to choose better name and better place 🤣
|
Thanks for getting this so fast, I was planning to look at it this morning. 👍 LOL. I did a review and it wouldn't let me submit because Niko left the same two comments while I was typing. 👍 |
Co-authored-by: Niko Oliveira <onikolas@amazon.com>
There was a problem hiding this comment.
I feel like it is quite trivial to move it to a more generic place (and still keep it private if you'd prefer that). Seems easy to make the right choice now rather than having to fix it later. But I wont press any harder, happy to merge this as is and I can fix it in a follow up PR 👍
Follow-up: #29761 (comment)