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 commands run on paths with danger-pr in them #1375

Conversation

mikedidomizio
Copy link
Contributor

This fixes an issue I encountered when I was running danger commands from a project that has danger-pr* in the project name.

The issue is because the replace searches and finds the first instance of danger-{command} in the path and replaces it with danger-runner. So when inside a project like danger-project-setup you end up with an error

node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module '/Users/Mike.DiDomizio/projects/test/danger-runneroject-setup/node_modules/danger/distribution/commands/danger-pr.js'

The fix I've implemented is to create a regex with the command, searching for the danger-${name}\.js$ at the end of the path and replace it. So we're covered for the other commands as well. Although I could possibly clean up the loop logic and just generate a regex that covers the array of commands, I've decided to keep this PR simple to make the change easy to approve.

The easiest way to reproduce this issue is to create a project like danger-project-setup and run the danger pr command inside the project.

@mikedidomizio mikedidomizio force-pushed the support-projects-with-danger-in-project-title branch from c920004 to ad73816 Compare April 8, 2023 16:33
@orta
Copy link
Member

orta commented Apr 8, 2023

Yeah, perfect 👍🏻

@orta orta merged commit cffbb1d into danger:main Apr 8, 2023
2 checks passed
@orta
Copy link
Member

orta commented Apr 8, 2023

Shipped as 11.2.5

@mikedidomizio mikedidomizio deleted the support-projects-with-danger-in-project-title branch April 9, 2023 22:53
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

2 participants