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.

Sign up for GitHub

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

Jump to bottom

Refactor isNullish from different package to js.common #2986

Merged
merged 1 commit into from Mar 23, 2023
Merged

Refactor isNullish from different package to js.common #2986

merged 1 commit into from Mar 23, 2023

Conversation

mstoykov
Copy link
Collaborator

No description provided.

@mstoykov mstoykov added this to the v0.44.0 milestone Mar 23, 2023
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (fixPromiseReturnsInSetupAndHandleSummary@0025ab3). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head e43f18c differs from pull request most recent head 66a29ef. Consider uploading reports for the commit 66a29ef to get more accurate results

@@                             Coverage Diff                             @@
##             fixPromiseReturnsInSetupAndHandleSummary    #2986   +/-   ##
===========================================================================
  Coverage                                            ?   76.86%           
===========================================================================
  Files                                               ?      228           
  Lines                                               ?    16985           
  Branches                                            ?        0           
===========================================================================
  Hits                                                ?    13056           
  Misses                                              ?     3086           
  Partials                                            ?      843
Flag Coverage Δ
ubuntu 76.86% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

na--
na-- previously approved these changes Mar 23, 2023
View reviewed changes
codebien
codebien reviewed Mar 23, 2023
View reviewed changes
js/runner.go
@@ -572,10 +572,6 @@ func unPromisify(v goja.Value) goja.Value {
return v
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we could move unpromisify in the common package also

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could ... but nothing else uses it - and I prefer to keep common and util packages smaller then bigger.

Arguably a lot of changes will happen around the ESM changes and some other module work - if it turns out this is used a lot we can move it then.

codebien
codebien previously approved these changes Mar 23, 2023
View reviewed changes
Base automatically changed from fixPromiseReturnsInSetupAndHandleSummary to master March 23, 2023 10:51
@mstoykov mstoykov dismissed stale reviews from codebien and na-- via 39e7489 March 23, 2023 10:54
@mstoykov mstoykov requested review from codebien and na-- March 23, 2023 10:54
codebien
codebien approved these changes Mar 23, 2023
View reviewed changes
Copy link
Collaborator

@codebien codebien left a comment

Choose a reason for hiding this comment

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

LGTM, restarted the flaky test

@mstoykov mstoykov merged commit 4f5c142 into master Mar 23, 2023
21 checks passed
@mstoykov mstoykov deleted the refactoIsNullish branch March 23, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codebien codebien codebien approved these changes

@na-- na-- na-- approved these changes

@oleiade oleiade Awaiting requested review from oleiade

Assignees
No one assigned
Labels
refactor
Projects
None yet
Milestone
v0.44.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mstoykov @codecov-commenter @codebien @na--