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

Add Skip & SkipLast Filter #12

Closed
wants to merge 1 commit into from
Closed

Add Skip & SkipLast Filter #12

wants to merge 1 commit into from

Conversation

pocket7878
Copy link
Contributor

Add Skip & SkipLast Filter

I'm using channel to buffering items in SkipLast.
But I'm wondering if I should use a queue. Could you give me some advice?

@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage increased (+1.3%) to 73.425% when pulling 9bb6b5c on pocket7878:skip into 51d692c on jochasinga:master.

@jochasinga
Copy link
Member

@pocket7878 thanks for your active contributions. I will check back asap. Meanwhile, before your next PR, can you open an issue for a feature you'd like to add so we can have some discussions there? Also, it might be a good idea to send your PR to the dev branch so we can keep master clean ;)

@pocket7878
Copy link
Contributor Author

@jochasinga That would be fine :)

@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage increased (+1.2%) to 74.672% when pulling 10ae37f on pocket7878:skip into e1b20f6 on jochasinga:master.

@jochasinga
Copy link
Member

@pocket7878 can you retract this PR, merge the latest tip back and revise the following:

  • rest_skip variable name is not following Go's convention, which requires variable names to be in camelCases like restSkip. However, itself is not descriptive. Can you think of a better variable name that better describe what it holds?

  • The parameter named skip in both Skip and SkipLast signature is not descriptive. Try nth

func Skip(nth uint) {
        // ...
}

then resubmit the PR again to the dev branch this time? I'd really appreciate this.

p.s. using a channel for buffering is fine, since channels are basically queues.

@jochasinga jochasinga closed this Feb 11, 2017
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.

None yet

3 participants