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 pattern based options #44

Merged

Conversation

jeffreyguenther
Copy link
Contributor

@jeffreyguenther jeffreyguenther commented Oct 2, 2021

In this PR, I fix a bug that's present in the action currently:

In Ruby, an empty string is truthy. As a result, passed a value of "" or "nil", these branches of the code will run. As a result, we need to change the default values of the input to "" and check if the passed value is an empty string. No parsing is done to the values passed in as args so these are just strings. "nil" doesn't become nil and is truthy.

In irb, you can check the following:

irb(main):024:1* if ""
irb(main):025:1*   puts "that string was truthy"
irb(main):026:0> end
(irb):26: warning: string literal in condition
that string was truthy
=> nil
irb(main):027:1* if "nil"
irb(main):028:1*   puts "that string was truthy"
irb(main):029:0> end
(irb):29: warning: string literal in condition
that string was truthy
=> nil
irb(main):030:1* if false
irb(main):031:1*   puts "that string was truthy"
irb(main):032:0> end
=> nil

In addition, I do some other clean up - make the option descriptions more consistent and restore an optimization that was recently removed.

We use a YAML nil `~` to provide a default of empty string.
In Ruby, an empty string is truthy. As a result, passed a value of "" or
"nil", these branches of the code will run.

As a result, we need to change the default values of the input to `""`
and check if the passed value is an empty string.

No parsing is done to the values passed in as args so these are just
strings. `"nil"` doesn't become `nil` and is truthy.
We want to make thse messages consistent and read in the positive.
"not empty" is harder to understand than "provided".

Also, let's match the description formats.
Let's use the same approach for matching the regex to make this file
easier future changes.

This approach uses ruby regex string interpolation, where you can turn a
string into a regex with interpolating it into a regex expression.
Let's skip retrieving the list of issue comments if none of the options
requiring them will be used.

This restores a previous optimization that was removed.
@jeffreyguenther jeffreyguenther marked this pull request as ready for review October 2, 2021 00:30
@jeffreyguenther
Copy link
Contributor Author

@aaronklaassen for your attention.

@wong2
Copy link

wong2 commented Oct 11, 2021

@aaronklaassen plz merge this, the action is broken now

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.

3 participants