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

Add exhaustive case matcher #43

Merged
merged 4 commits into from
Jan 7, 2024
Merged

Add exhaustive case matcher #43

merged 4 commits into from
Jan 7, 2024

Conversation

peterfication
Copy link
Collaborator

@peterfication peterfication commented Jan 5, 2024

Can be used by including Ruby::Enum::Ecase in an enum class. It will add a method called ecase that can be used to simulate a case statement that will raise an error if a case/enum value is not handled.

Fixes #42

@peterfication
Copy link
Collaborator Author

@dblock here you go :)

Copy link
Owner

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Very close! I left a bunch of comments/questions, take a look.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/ruby-enum/enum/ecase.rb Outdated Show resolved Hide resolved
lib/ruby-enum/enum/ecase.rb Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
spec/ruby-enum/enum/ecase_spec.rb Outdated Show resolved Hide resolved
@peterfication
Copy link
Collaborator Author

Thanks a lot for the deep review and your comments! I appreciate it :)

@dblock
Copy link
Owner

dblock commented Jan 6, 2024

Make rubocop happy and we're good to go! Much thanks.

@dblock
Copy link
Owner

dblock commented Jan 6, 2024

Oh and since it's a new feature, increment the version to 0.10.0 in this PR. I also wonder whether it's time for Ruby::Enum to graduate to 1.0. That works for me too, your call!

@peterfication
Copy link
Collaborator Author

Rubocop is failing on master. Hence, I will make the Rubocop adjustments in a separate commit.

Can be used by including `Ruby::Enum::Ecase` in an enum class. It will
add a method called `ecase` that can be used to simulate a case
statement that will raise an error if a case/enum value is not handled.
@peterfication
Copy link
Collaborator Author

I had to disable and ignore some Rubocop rules. See the Rubocop related commit. I wanted to make as little changes as possible in that commit.

@peterfication
Copy link
Collaborator Author

I feel like it's a good idea to release a 1.0.0 version as it has been proven stable.

Note: the 0.9.1 version has not been tagged or released to Ruby gems yet.

@peterfication
Copy link
Collaborator Author

But maybe it makes sense to wait for #41 for the 1.0.0 release.

Copy link
Owner

@dblock dblock left a comment

Choose a reason for hiding this comment

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

For Rubocop, you can remove any of the manual disable and the new rules and just run rubocop -a ; rubocop --auto-gen-config, but what you have here works fine too.

CHANGELOG.md Outdated Show resolved Hide resolved
@dblock
Copy link
Owner

dblock commented Jan 7, 2024

But maybe it makes sense to wait for #41 for the 1.0.0 release.

Are you going to take it up? :) Please!

@peterfication
Copy link
Collaborator Author

Are you going to take it up? :) Please!

I might give it a try next week.

@dblock dblock merged commit c38afd8 into dblock:master Jan 7, 2024
9 checks passed
@dblock
Copy link
Owner

dblock commented Jan 7, 2024

Great work @peterfication! Want to help comantain ruby-enum? Drop me your rubygems username to dblock at dblock dot org!

@peterfication peterfication deleted the add-exhaustive-case-statement branch January 8, 2024 04:31
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.

[Feature request] Add exhaustive case matcher
2 participants