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

Generate rack-test.gemspec from Thorfile adding license, updating homepage #166

Merged
merged 1 commit into from
May 8, 2017

Conversation

junaruga
Copy link
Contributor

@junaruga junaruga commented May 8, 2017

  • Added license information in Thorfile. Right now the gemspec file has 2 license parts in the file.
    Also the license information would be better to be included in Thorfile. And the gemspec file should be generated from Thorfile by thor command.
    This fixes Explicitly set license to MIT in .gemspec #130
  • Updated homepage in Thorfile.

I used Ruby 2.2.7 to use thor.
Because when I used Ruby 2.4, the strings part of the gemfile was generated as "foo".freeze.
That is hard to check the difference of last modified one.

Below is how to generate.

$ which ruby
/usr/local/ruby-2.2.7/bin/ruby

$ ruby -v
ruby 2.2.7p470 (2017-03-28 revision 58193) [x86_64-linux]

$ which bundle
/usr/local/ruby-2.2.7/bin/bundle

$ bundle -v
Bundler version 1.14.6

$ bundle install --path vendor/bundle

$ bundle exec thor :gemspec
Wrote gemspec to rack-test.gemspec
WARNING:  open-ended dependency on rack (>= 1.0) is not recommended
  if rack is semantically versioned, use:
    add_runtime_dependency 'rack', '~> 1.0'
WARNING:  See http://guides.rubygems.org/specification-reference/ for help

@junaruga junaruga force-pushed the feature/generate-gemspec branch from 3145a30 to 7b29932 Compare May 8, 2017 17:26
@junaruga junaruga changed the title Generate rack-test.gemspec from Thorfile adding license. Generate rack-test.gemspec from Thorfile adding license, updating homepage May 8, 2017
@junaruga junaruga mentioned this pull request May 8, 2017
Copy link
Contributor

@perlun perlun left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

Again, this is an area I think it would be fine to change. I have never encountered a gem before where the gemspec is maintained via a Thorfile. It feels unnatural to me. The gemspec is more a file that you maintain manually, in my book, so I think we could move over to that pattern if you and/or @scepticulous agree. I don't really see the value of having it be machine-generated, when we could just as well let it be the "single source of truth" for its content instead.

Copy link

@dennissivia dennissivia left a comment

Choose a reason for hiding this comment

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

I agree with @perlun about potentially removing thor from this equation. However I would actually merge this change the way it is, make the release.
Next we can discuss cleanup and removing things we no longer want/need.

@junaruga
Copy link
Contributor Author

junaruga commented May 8, 2017

@perlun @scepticulous I agree with you guys.
I like maintaining the gemspec file manually. I like not using thor.

Next we can discuss cleanup and removing things we no longer want/need.

Yes, I agree the way.

* Add license.
* Update Home Page
@junaruga junaruga force-pushed the feature/generate-gemspec branch from 7b29932 to 9a7cfa4 Compare May 8, 2017 20:18
@junaruga
Copy link
Contributor Author

junaruga commented May 8, 2017

Thanks for the reviewing!

@junaruga junaruga merged commit 8fc7790 into rack:master May 8, 2017
@junaruga junaruga mentioned this pull request May 8, 2017
@@ -1,25 +1,26 @@
# -*- encoding: utf-8 -*-
# stub: rack-test 0.6.3 ruby lib
Copy link
Contributor

Choose a reason for hiding this comment

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

This line makes no sense. This is autogenerated by RubyGems for .gemspec files used after installation. Moreover, it is not visible on GH, but the separator is null character, not sure what separator is used here ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@voxik thank you for mentioning it. Yes, this PR includes the gemspec file generated from Thorfile by thor. The line will be removed by PR: #181 .

Copy link
Contributor Author

@junaruga junaruga May 30, 2017

Choose a reason for hiding this comment

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

I ran thor command on Ruby 2.2.7 when I generated the gemspec file. The Ruby's RubyGems was used to append the line. Maybe.
Because when I tried it on Ruby 2.4, freeze string was appended in the gemspec file. and there was so much difference between before and after modification. I did not like the difference.

alex-damian-negru pushed a commit to alex-damian-negru/rack-test that referenced this pull request Apr 5, 2021
* Add license.
* Update Home Page
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.

4 participants