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

Feature/fix lookup #1674

Merged
merged 4 commits into from
Mar 29, 2022
Merged

Feature/fix lookup #1674

merged 4 commits into from
Mar 29, 2022

Conversation

yunnysunny
Copy link
Contributor

It's the fix of the pull request #1595

I also found that there were some error after running make test, some related to the wrong configuration of eslint, and others related to testcases. I will make another pull request to fix this errors.

And I think it's important to enable the ci check before any merges to aviod bugs coming from new code.

@niftylettuce
Copy link
Collaborator

Are you sure this is all good to go now?

@yunnysunny
Copy link
Contributor Author

Yes, it's ok. You can first merge the request of #1677 , and then merge this, you will see all testcases is ok.

@titanism titanism merged commit 31e0160 into ladjs:master Mar 29, 2022
@titanism
Copy link
Collaborator

@jmcollin78
Copy link

Hello @titanism , the release 7.1.2 seems to not exists yet. I'm waiting for the lookup feature and I was glad to see there was a fix for this.
What about tag 7.1.2 ?

@titanism
Copy link
Collaborator

titanism commented Apr 6, 2022

v7.1.2 was unpublished from npm and GitHub as it failed to pass all tests in CI across all Node versions.

Do we need to merge both #1716 and #1717 @yunnysunny? Also see #1717 (comment) (ref #1677 (comment))

@yunnysunny
Copy link
Contributor Author

v7.1.2 was unpublished from npm and GitHub as it failed to pass all tests in CI across all Node versions.

Do we need to merge both #1716 and #1717 @yunnysunny? Also see #1717 (comment) (ref #1677 (comment))

Try to merge #1716 , and see if the error is missing.

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