Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rewrote "bad" Code to more modern version #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rewrote "bad" Code to more modern version #108

wants to merge 1 commit into from

Conversation

melvin-suter
Copy link
Contributor

As discussed in #100 here is the PR with the rewroten if/else statement.

@melvin-suter
Rewrote "bad" Code from "goto and multiple ifs" to a more modern veri…
2c3c61c 
…son with "switch-case"

Signed-off-by: Melvin Suter <github@damon.ch>
ufozone
ufozone approved these changes Sep 24, 2019
View reviewed changes
Copy link

@ufozone ufozone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

@violoncelloCH
Copy link
Member

thank you very much for your contribution!
indeed, your changes make sense
however this needs proper testing in different scenarios before merging, but unfortunately I neither have an SMB setup nor enough time to do this at the moment :/

@violoncelloCH violoncelloCH added 3. to review enhancement New feature or request labels Oct 5, 2019
@violoncelloCH
Copy link
Member

@ufozone simply "accepting" the PR doesn't really help here, because I don't know what this "accepting" is based on. Instead please test it as much as possible and explain what you tested in detail :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ufozone ufozone ufozone approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
3. to review enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@melvin-suter @violoncelloCH @ufozone