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

Added next, pick and negate. #53

Closed
wants to merge 8 commits into from
Closed

Added next, pick and negate. #53

wants to merge 8 commits into from

Conversation

jvduf
Copy link
Contributor

@jvduf jvduf commented Jan 8, 2015

Based on @HenrikJoreteg 's array-next module.

@wraithgar
Copy link
Contributor

Hi @jvduf I have a sort of out-of-band question regarding this PR. If you don't have the time please disregard.

I was wondering if you made this PR using the add-module script. If so, how easy was it for you to discover and how easy was it to use? Were the contribution instructions easy enough to find?

Essentially, since you're the first to submit a new module other than Henrik and I, I'd really really like your first impressions about the process. Be as critical as you feel comfortable, your first impression is important and can only help to make this project better.

@jvduf
Copy link
Contributor Author

jvduf commented Jan 8, 2015

@wraithgar I've got some more coming :)

Yes, I used the add-module script.

It was pretty straight forward. The only thing unclear to me was the sig file. But I looked at the other modules to see what it was used for and I figured it out :),

@HenrikJoreteg
Copy link
Member

Thanks for doing this! It was one of the utils I was planning on including anyway :)

On Thu, Jan 8, 2015 at 2:33 PM, Jeroen van Duffelen
notifications@github.com wrote:

@wraithgar I've got some more coming :)

It was pretty straight forward. The only thing unclear to me was the sig file. But I looked at the other modules to see what it was used for and I figured it out :)

Reply to this email directly or view it on GitHub:
#53 (comment)

@jvduf
Copy link
Contributor Author

jvduf commented Jan 8, 2015

@wraithgar @HenrikJoreteg the CI build failed as a result of Zuul tried to run tests in saucelabs, however no saucelabs credentials were provided.

@bear
Copy link
Contributor

bear commented Jan 8, 2015

@jvduf the travis config is working for saucelabs, if you update your PR you should see test results

@wraithgar
Copy link
Contributor

+1 tests run locally for me

"name": "amp-next",
"description": "next function part of http://amp.ampersandjs.com.",
"version": "1.0.0",
"author": "Henrik Joreteg <henrik@andyet.net>",
Copy link
Member

Choose a reason for hiding this comment

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

Should the author in this case be "Jeroen van Duffelen"? Or prefer to keep them all as @HenrikJoreteg?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should change it so they're not all me, that's definitely a bit silly. However, in this particular case he just dropped in my module: https://www.npmjs.com/package/array-next

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I don't really care :) Just added @HenrikJoreteg because it was his original code anyway.

HenrikJoreteg added a commit that referenced this pull request Jan 9, 2015
@jvduf jvduf changed the title Added next utility to fetch the next item in an array. Added next, pick and negate. Jan 13, 2015
This was referenced Jan 13, 2015
@jvduf jvduf closed this Jan 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants