-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix dry run bugs #317
Fix dry run bugs #317
Conversation
Codecov Report
@@ Coverage Diff @@
## master #317 +/- ##
============================================
- Coverage 87.14% 87.12% -0.02%
- Complexity 930 933 +3
============================================
Files 112 112
Lines 2427 2455 +28
Branches 279 283 +4
============================================
+ Hits 2115 2139 +24
- Misses 233 235 +2
- Partials 79 81 +2
Continue to review full report at Codecov.
|
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.
The GUI works as expected and the test passes as well! LGTM once you're done with the tweaks
…resets the ordering of client list to its default
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, suggestions aldy implemented based on Telegram chats 👍
checked mainly from the GUI and gave the code a cursory look. everything based on issues work, couldn't break anything so looks good
Description
Fixes #279, fixes #290, fixes #299, fixes #301, fixes #303, fixes #305, fixes #306, fixes #313
Testing
Ran tests, all pass. Tried the bugs stated also, seems alright
Remarks
For #301 and #303, the full name/addresses are only viewable in the display panel, will create a new issue to add this info into the UG
I also made the
list
andsuggest
commands clear the current viewed client in the display panel