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 docs generator when git executable is missing #9867

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Oct 30, 2020

The code for executing the git program didn't take into account that Process.run raises if the executable can't be found. This is now fixed. I also refactored the code for capturing the program output (DRY).

Fixes #9789

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

Should we do the same on crystal init later?

@bcardiff bcardiff added this to the 1.0.0 milestone Nov 6, 2020
@straight-shoota
Copy link
Member Author

I noticed there are similar cases, but I just focused on the more common problem here. I'll make another PR for init.

@straight-shoota
Copy link
Member Author

Given that it's quite common to ignore failing programs (like status = Process.run("command"); return unless status.success?), maybe we should consider a variant Process.run? that doesn't raise on exec error. It could just return nil which is still easier to handle than to rescue IO::Error. Alternatively, perhaps Process::Status could be extended for this use case, so that the above example code would (with Process.run?) just work even if the command can't be executed.

@bcardiff bcardiff removed this from the 1.0.0 milestone Nov 10, 2020
@bcardiff
Copy link
Member

Closing in favor of #9893

@bcardiff bcardiff closed this Nov 10, 2020
@straight-shoota straight-shoota deleted the fix/docs-generator-git-missing branch November 10, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:refactor topic:tools:docs-generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing assertion error when building docs without git installed
3 participants