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.

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

Code cleanup. Deleted unnecessary arguments #17

Closed
wants to merge 1 commit into from

Conversation

blasco
Copy link

@blasco blasco commented Oct 10, 2019

No description provided.

@blasco
Copy link
Author

blasco commented Oct 10, 2019

Just some code cleanup, I would also like to add support for the dash delimiters and user delimiters. Working on it

@Julian
Copy link
Owner

Julian commented Oct 10, 2019

Hi, thanks so much.

Definitely would love some help on the delimiter side -- very keen to hear what you have in mind.

On this change specifically though, less sure I see the benefit. The original code here had no conditionals, whereas the new code has quite a few (or really branches). Which in general I think is the reverse direction from what I'd want. Happy to hear thoughts of course.

@blasco
Copy link
Author

blasco commented Oct 19, 2019

I see your point. For me it was clearer to read like this. Regarding the --, I still didn't find the time. I wish I had it a few times during the week though hahaha so it might worth putting the effort. I'll let you know if I achieve any progress on this.

@Julian
Copy link
Owner

Julian commented Oct 24, 2019

No worries.

Going to close this PR specifically then, but your thoughts or contribution on #7 would be awesome!

@Julian Julian closed this Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants