Skip to content

Rubyspec: Array#combination#848

Merged
vais merged 3 commits intoopal:masterfrom
c42engineering:rubyspec_array_combination
May 14, 2015
Merged

Rubyspec: Array#combination#848
vais merged 3 commits intoopal:masterfrom
c42engineering:rubyspec_array_combination

Conversation

@kaiwren
Copy link
Copy Markdown
Contributor

@kaiwren kaiwren commented May 12, 2015

No description provided.

Sidu Ponnappa added 2 commits May 12, 2015 02:36
…lock is passed but not when an Enumerable#to_a is called.
Replaced self.dup with `self.slice()`
Moved var declarations to the top to reflect hoisting
@vais
Copy link
Copy Markdown
Contributor

vais commented May 12, 2015

Excellent, @kaiwren! But one last small problem: the variable length you use in your for loops is never defined, creating a global variable leak.

Side note: We really should find a way to integrate JSLint/JSHint into the build/test process for Opal at some point.
cc @elia @adambeynon @meh

@elia
Copy link
Copy Markdown
Member

elia commented May 13, 2015

@vais I can see it added to allow failure in travis with something like this:

rake dist
jslint build/*

I don't expect it to be green for a while…

@elia
Copy link
Copy Markdown
Member

elia commented May 13, 2015

probably this is a good option: https://github.com/stereobooster/jshintrb
would be cool (maybe) to have it optionally enabled at compile time…

@elia
Copy link
Copy Markdown
Member

elia commented May 13, 2015

@vais
Copy link
Copy Markdown
Contributor

vais commented May 14, 2015

@kaiwren I'd like to merge this PR, could you do another push with var length declared?

@kaiwren
Copy link
Copy Markdown
Contributor Author

kaiwren commented May 14, 2015

Apologies, fixed.

@vais
Copy link
Copy Markdown
Contributor

vais commented May 14, 2015

🍻

vais added a commit that referenced this pull request May 14, 2015
@vais vais merged commit ba56969 into opal:master May 14, 2015
elia added a commit that referenced this pull request May 15, 2015
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.

4 participants