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

Pull request challange - small improvements #22

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@happy-barney

happy-barney commented Mar 27, 2018

No description provided.

@Tux

Tux approved these changes Mar 28, 2018

Will be in. Thank you

@Tux

Rejected. This is not the same. No objections to using first, but first does not remove the argument from the list

@Tux

This comment has been minimized.

Owner

Tux commented Mar 28, 2018

  1. Typoes: thank you, applied and pushed
  2. List::Util. Will have to think about that one. It is elegant, but does not do the same as the original code. The original code removes the file argument from the list. The list is later used in the last call, so that removal is vital.
  3. Support input option: I like it. Applied and pushed
  4. Perl version. That needs a lot more testing. I had something similar in before, but something broke, so there will be or might have been situations where that didn't work
  5. No new tests?

Thanks for your interest

@happy-barney

This comment has been minimized.

happy-barney commented Mar 28, 2018

two commits applied, two no so good

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