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

Fix generated bin doesn't work #970

Merged
merged 2 commits into from
Jan 5, 2020
Merged

Fix generated bin doesn't work #970

merged 2 commits into from
Jan 5, 2020

Conversation

HelloCore
Copy link
Member

Fix #958

When spawning child process from pkg we got argv like this.

[ '/Users/core/Documents/project/danger-js/brew-distribution/danger',
  '/snapshot/danger-js/distribution/commands/danger-pr.js',
  'https://bitbucket.org/foo/bar/pull-requests/381' ]

However, it got converted to the wrong command.
Moreover, there was another bug about the child process that couldn't resolve dangerfile.ts path, so upgrading the pkg's version fixed this.

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

Cool, thank you!

@f-meloni
Copy link
Member

f-meloni commented Jan 4, 2020

I've tried to generate the bin from this branch, but I've got

brew-distribution/danger pr https://github.com/danger/danger-js/pull/956

Starting Danger PR on danger/danger-js#956
Unable to evaluate the Dangerfile
 TypeError: process.binding(...).internalModuleReadFile is not a function
    at Object.fs.internalModuleReadFile.fs.internalModuleReadJSON (pkg/prelude/bootstrap.js:1127:36)
    at internalModuleReadJSON (internal/modules/cjs/loader.js:31:68)
    at readPackage (internal/modules/cjs/loader.js:156:16)
    at tryPackage (internal/modules/cjs/loader.js:172:13)
    at Function.Module._findPath (internal/modules/cjs/loader.js:282:18)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:589:25)
    at Function.Module._resolveFilename (pkg/prelude/bootstrap.js:1270:44)
    at Function.Module._load (internal/modules/cjs/loader.js:518:25)
    at Module.require (internal/modules/cjs/loader.js:648:17)
    at Module.require (pkg/prelude/bootstrap.js:1157:31)

@HelloCore
Copy link
Member Author

I've tried to generate the bin from this branch, but I've got

brew-distribution/danger pr https://github.com/danger/danger-js/pull/956

Starting Danger PR on danger/danger-js#956
Unable to evaluate the Dangerfile
 TypeError: process.binding(...).internalModuleReadFile is not a function
    at Object.fs.internalModuleReadFile.fs.internalModuleReadJSON (pkg/prelude/bootstrap.js:1127:36)
    at internalModuleReadJSON (internal/modules/cjs/loader.js:31:68)
    at readPackage (internal/modules/cjs/loader.js:156:16)
    at tryPackage (internal/modules/cjs/loader.js:172:13)
    at Function.Module._findPath (internal/modules/cjs/loader.js:282:18)
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:589:25)
    at Function.Module._resolveFilename (pkg/prelude/bootstrap.js:1270:44)
    at Function.Module._load (internal/modules/cjs/loader.js:518:25)
    at Module.require (internal/modules/cjs/loader.js:648:17)
    at Module.require (pkg/prelude/bootstrap.js:1157:31)

I think you need to run yarn install again.

@f-meloni
Copy link
Member

f-meloni commented Jan 4, 2020

Thank you!
Worked :)

@orta
Copy link
Member

orta commented Jan 5, 2020

Looks good to me - thanks for the test @HelloCore, solid work

@orta orta merged commit 2f13081 into danger:master Jan 5, 2020
@peril-staging
Copy link
Contributor

peril-staging bot commented Jan 5, 2020

Thanks for the PR @HelloCore.

This PR has been shipped in v9.2.10 - CHANGELOG.

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.

Generated bin for Brew doesn't work
3 participants