Skip to content

fix resolver order#433

Open
luxzeitlos wants to merge 2 commits into
denali-js:masterfrom
luxzeitlos:fix-resolver-order
Open

fix resolver order#433
luxzeitlos wants to merge 2 commits into
denali-js:masterfrom
luxzeitlos:fix-resolver-order

Conversation

@luxzeitlos

@luxzeitlos luxzeitlos commented Feb 18, 2018

Copy link
Copy Markdown
Contributor

Fix wrong used .reverse() in availableForType

beware: .reverse() does modify the original array! This can be prevented by creating a new array with [...arr].

Fixes #432

@luxzeitlos luxzeitlos force-pushed the fix-resolver-order branch 2 times, most recently from 5915871 to 35aa4c4 Compare February 18, 2018 22:34

@davewasmer davewasmer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I can haz test plz?

@davewasmer

Copy link
Copy Markdown
Collaborator

Good catch! Any chance you'd be able to add a test for this?

@luxzeitlos

Copy link
Copy Markdown
Contributor Author

haz test now 🍵

@luxzeitlos

Copy link
Copy Markdown
Contributor Author

Whats the status here? this has been fixed on master independent of this PR. Maybe close the PR, or should I rebase to just take the test?

@knownasilya

Copy link
Copy Markdown
Member

Not sure if this is still needed. Looks like the reverse was removed from master.

@knownasilya

Copy link
Copy Markdown
Member

See b4ae232. Although tests weren't added there.

@knownasilya

Copy link
Copy Markdown
Member

Removed the fix but kept the test. Will see if it passes haha.

@knownasilya

Copy link
Copy Markdown
Member

Hum. Looks like travis isn't set up?

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