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

Expose Date::VERSION #42

Merged
merged 1 commit into from
Nov 17, 2021
Merged

Expose Date::VERSION #42

merged 1 commit into from
Nov 17, 2021

Conversation

casperisfine
Copy link

An almost universal convention for gems is to expose Namespace::VERSION
which makes it mcuh easier when debugging etc.

Many gems extracted from ruby don't do this, even though it would be even more
useful because they ship with ruby, so it's less clear which version it is.

cc @mame

@@ -4,6 +4,7 @@
require 'date_core'

class Date
VERSION = '3.2.1' # :nodoc:
Copy link
Author

Choose a reason for hiding this comment

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

In the normal pattern it would be defined in lib/date/version.rb and required in the gemspec, but I'm not sure what the consequence could be for something like Date. So I do some parsing instead.

@mame mame requested a review from hsbt November 16, 2021 15:06
Copy link
Member

@mame mame left a comment

Choose a reason for hiding this comment

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

I think it is a good-to-have. AFAIK, adding VERSION to default gems could be tricky because it must work not only on gem repository but also ruby/ruby repository. This change looks reasonable, but I am not familiar, so I ask @hsbt to review.

@hsbt
Copy link
Member

hsbt commented Nov 17, 2021

@casperisfine Seems fine. But It's good to separate this detection logic into outside of gemspec block like https://github.com/ruby/net-http/blob/master/net-http.gemspec#L4

Can you update this pull request like net-http.gemspec? Thanks.

@casperisfine
Copy link
Author

Of course.

An almost universal convention for gems is to expose `Namespace::VERSION`
which makes it mcuh easier when debugging etc.

Many gems extracted from ruby don't do this, even though it would be even more
useful because they ship with ruby, so it's less clear which version it is.
@casperisfine
Copy link
Author

Done.

@hsbt
Copy link
Member

hsbt commented Nov 17, 2021

@casperisfine Thanks!

@hsbt hsbt merged commit d5ddd76 into ruby:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants