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 demo for Array.prototypes Fixes #417 and #416 #439

Merged
merged 11 commits into from
Jan 22, 2018
Merged

Add demo for Array.prototypes Fixes #417 and #416 #439

merged 11 commits into from
Jan 22, 2018

Conversation

maddhruv
Copy link
Contributor

No description provided.

@maddhruv maddhruv changed the title Add demo for Array.some() Fixes #417 Add demo for Array.prototypes Fixes #417 and #416 Jan 21, 2018
Copy link
Contributor

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

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

Couple of items that needs attention in Array.some(), but this is looking good. Thanks @maddhruv


var even = function(element, index, array) {
// checks whether any element is even
return element % 2 == 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use === here.

<pre>
<code id='static-js'>var array = [1, 2, 3, 4, 5];

var even = function(element, index, array) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You are not using index nor array inside the function so, no need to declare them as parameters.

<code id='static-js'>var array = [1, 2, 3, 4, 5];

var even = function(element, index, array) {
// checks whether any element is even
Copy link
Contributor

Choose a reason for hiding this comment

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

// checks whether an element is even

@schalkneethling
Copy link
Contributor

schalkneethling commented Jan 22, 2018

Thanks @maddhruv for the update. Not sure how familiar you are with Git. Would you be ok with squashing(rebasing) your commits down to a single commit?

@schalkneethling
Copy link
Contributor

@maddhruv No worries, I see you worked directly on your master branch. This might be a little prickly and I do not want you to loose your work. In future, please create a feature branch instead of working directly on master.

https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow

@schalkneethling schalkneethling merged commit 05b5a31 into mdn:master Jan 22, 2018
@maddhruv
Copy link
Contributor Author

Thanks, @schalkneethling for your support. I actually didn't rebase

@wbamberg
Copy link
Contributor

Would you be ok with squashing(rebasing) your commits down to a single commit?

@schalkneethling , I just use the "Squash and merge" big green button, so contributors don't have to squash commits. Is there a reason I shouldn't be doing this?

@schalkneethling
Copy link
Contributor

Is there a reason I shouldn't be doing this?

Nope. If the person is used to doing it with Git its a quick thing. Also, trying to guide people in the direction of rebasing once they have made final changes and expect a r+

Definitely not something that should block anyone from using the Github squash and merge and holding up a PR

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.

3 participants