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

Core: Node API was not calling right method #41

Merged
merged 1 commit into from
May 25, 2020
Merged

Conversation

tooppaaa
Copy link
Collaborator

No description provided.

@tooppaaa tooppaaa requested a review from obahareth May 21, 2020 18:10
@tooppaaa
Copy link
Collaborator Author

Previous one was merged a bit too fast ;)

@obahareth
Copy link
Owner

obahareth commented May 25, 2020

@tooppaaa We should release a new version for this I assume, right? 2.0.1?
Sorry for the fast merge before, I am a bit flooded with work at the moment and I thought the repo in the state it was before was blocking you from using as a library so I wanted to cooperate fast. I will do more thorough reviews from now on.

@obahareth
Copy link
Owner

By the way @tooppaaa can you share your npmjs.com username with me? I'd like to add you to the package on there as well.

@tooppaaa
Copy link
Collaborator Author

Hey @obahareth no worries :)
My username is the same as github: tooppaaa

Yes, we can release :)

@tooppaaa tooppaaa merged commit 448c332 into master May 25, 2020
@tooppaaa tooppaaa deleted the fix/checModules branch May 25, 2020 07:38
@varoot
Copy link

varoot commented May 29, 2020

Can we release this and also update the documentation on README to remove reference to result.es6Modules?

@tooppaaa
Copy link
Collaborator Author

@varoot Sorry I didn't get (saw !?) a notification when I was added to npm for this package.
This is (should) be deployed.

Could you please let me know as it's my first time publishing this package if it's all good on your side ?
Thanks a lot !!

@varoot
Copy link

varoot commented May 31, 2020

Could you please let me know as it's my first time publishing this package if it's all good on your side ?

@tooppaaa Yep. It's working now. Thanks a lot!

@obahareth
Copy link
Owner

obahareth commented May 31, 2020

Can we release this and also update the documentation on README to remove reference to result.es6Modules?

@tooppaaa @varoot I'm a little confused on this part, I think we need this? The functions that build the regex expect an array of strings and that should be result.es6Modules, no? Because result is just an object, e.g. see #39 (which isn't released yet, but perhaps we'll change it to match what's expected here to avoid breaking anything).

@varoot
Copy link

varoot commented Jun 1, 2020

Hmm... I think there must be another PR to change this behavior. Previously it was returning array of strings.

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.

None yet

3 participants