search

dyedurham / SbManager

Installable web ui for managing Azure/Windows Service Bus
Apache License 2.0
31 stars 16 forks source link

Add a new route to show only queues/topics containing deadletters #84

Closed crejb closed 7 years ago

crejb commented 7 years ago

An alternative to https://github.com/GlobalX/SbManager/pull/79 is to add a new route '/deadletters' to show only the queues/topics containing deadletters. The home screen is not changed. (Good suggestion from @drewfreyling )

jonyeezs commented 7 years ago

It does what it suppose to do which is acceptable.

But for the future, this isn't a good pattern to follow when adding new features and could set a bad precedent for future contributors. The home screen shouldn't be a list of if statements. Ideally, any reusable parts should be a shared directive.

Indeed, can be tempting to just modify the existing file.

drewfreyling commented 7 years ago

I'm thinking we should remove the ability to remove all queues and topics from the dead letter screen as you would be removing stuff you can't see. What do you think @crejb and @jonyeezs ?

crejb commented 7 years ago

@jonyeezs understood about SRP and componentising, was going for a more YAGNI approach. As per @drewfreyling 's suggestion, I've removed the action buttons so it makes more sense now to separate the page as there is not much shared code now. Still sharing the same controller though.

jonyeezs commented 7 years ago

@crejb fair enough. Just something to keep in mind.

LGTM