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

Tadpole operator to String#match? #802

Merged
merged 3 commits into from
Mar 2, 2020
Merged

Tadpole operator to String#match? #802

merged 3 commits into from
Mar 2, 2020

Conversation

hachi8833
Copy link
Member

This is just my idea that the tadpole operator =~ looks cryptic and is a legacy from Perl, so changing this to match? is better for clarification.

Rubocop style guides also suggests that =~ is not (partially) recommended: https://github.com/rubocop-hq/ruby-style-guide#no-perl-regexp-last-matchers

If unnecessary, just discard the PR.
Thank you,

@codecov
Copy link

codecov bot commented Nov 16, 2019

Codecov Report

Merging #802 into master will increase coverage by 0.65%.
The diff coverage is 61.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #802      +/-   ##
==========================================
+ Coverage   80.26%   80.91%   +0.65%     
==========================================
  Files          54       54              
  Lines        7397     7419      +22     
==========================================
+ Hits         5937     6003      +66     
+ Misses       1235     1191      -44     
  Partials      225      225
Impacted Files Coverage Δ
vm/object.go 80% <ø> (ø) ⬆️
native/db/db.go 61% <ø> (ø) ⬆️
vm/error.go 70.96% <ø> (ø) ⬆️
vm/stack.go 88.57% <ø> (ø) ⬆️
vm/validate.go 54.48% <ø> (ø) ⬆️
vm/regexp.go 75.51% <ø> (ø) ⬆️
vm/instruction.go 89.97% <ø> (ø) ⬆️
vm/hash.go 96.88% <ø> (ø) ⬆️
vm/thread.go 87.22% <ø> (+1.13%) ⬆️
vm/channel.go 84.61% <ø> (-0.3%) ⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6e5979...73fad60. Read the comment docs.

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

@hachi8833 this looks great! thanks for the work! But I will hold the merge a little longer because it's a breaking change.

@st0012 st0012 added this to the version 0.2.0 milestone Nov 17, 2019
@hachi8833
Copy link
Member Author

No problem! I leave this to you.

Copy link
Member

@64kramsystem 64kramsystem left a comment

Choose a reason for hiding this comment

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

Rubocop style guides also suggests that =~ is not (partially) recommended: https://github.com/rubocop-hq/ruby-style-guide#no-perl-regexp-last-matchers

My understanding of that guide section is that it suggests not to use numbered captures - the expression with the tadpole (/(regexp)/ =~ string) is just something they use as example.

Regardless of this, I'm very used to the operator, but I also use Perl a lot 😂, so 👍 for the change.

@st0012
Copy link
Member

st0012 commented Mar 1, 2020

@hachi8833 I think this can go out in version 0.1.13. Will merge when build runs green.

@st0012 st0012 modified the milestones: version 0.2.0, version 0.1.13 Mar 1, 2020
@st0012 st0012 merged commit 5f0d187 into master Mar 2, 2020
@st0012 st0012 deleted the tadpole_to_match branch March 2, 2020 05:40
@hachi8833
Copy link
Member Author

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants