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

Remove check for "puts" when parsing Dangerfile #1363

Open
lukeredpath opened this issue Apr 21, 2022 · 4 comments
Open

Remove check for "puts" when parsing Dangerfile #1363

lukeredpath opened this issue Apr 21, 2022 · 4 comments

Comments

@lukeredpath
Copy link

lukeredpath commented Apr 21, 2022

This check is pretty annoying and naive - it will fail if I have a variable called inputs for example.

Also, I want to use puts in my Dangerfile to print out information to help debug and test my Dangerfile scripts in both local testing and CI output.

ui.puts "You used `puts` in your Dangerfile. To print out text to GitHub use `message` instead"

@orta
Copy link
Member

orta commented Apr 21, 2022

I'm open to an env var hiding the output

lukeredpath added a commit to lukeredpath/danger that referenced this issue Apr 21, 2022
Allows this warning message to be disabled by setting the ENV var `DANGER_DISABLE_PUTS_WARNING`.

This can be useful if a variable is triggering this accidentally or you just want to use `puts` for debug output.

Fixes danger#1363
@lukeredpath
Copy link
Author

Potential fix here: master...lukeredpath:patch-1

However, perhaps a better solution would be for the DSL to provide some kind of debug function that acts as a proxy to puts that won't get picked up by this check. That way you wouldn't be calling it by accident but its there if you want to output some debug statements to STDOUT.

@manicmaniac
Copy link
Member

For debugging purpose, you can use p or print instead of puts in order to avoid warnings.

IMO if you really want to use puts, introducing a new environment variable would be the first option.

@manicmaniac
Copy link
Member

manicmaniac commented Jan 30, 2023

BTW I agree with you that the current implementation is a bit naive and it may cause false-positive.

My rough idea is:

  1. Properly parsing contents with Ruby language parser (e.g. Ripper)
  • pros: it doesn't break anything
  • cons: overengineering
  1. Overriding Danger::Dangerfile#puts
  • pros: easy
  • cons: may break existing plugins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants