Skip to content

Commit

Permalink
Prepare for version 0.18.0
Browse files Browse the repository at this point in the history
  • Loading branch information
@sferik
sferik committed Mar 26, 2013
1 parent 8452893 commit 08265a3
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 0.18.0, release 2013-03-26
* Remove rake2thor
* Only display colors if output medium supports colors
* Pass parent_options to subcommands
* Fix non-dash-prefixed aliases
* Make error messages more helpful
* Rename "task" to "command"
* Add the method to allow for custom package name

## 0.17.0, release 2013-01-24
* Add better support for tasks that accept arbitrary additional arguments (e.g. things like `bundle exec`)
* Add #stop_on_unknown_option!
Expand Down
2 changes: 1 addition & 1 deletion lib/thor/version.rb
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
class Thor
VERSION = "0.17.0"
VERSION = "0.18.0"
end

10 comments on commit 08265a3

@rafaelfranca
Copy link
Member

Choose a reason for hiding this comment

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

This release broke the railties tests with Ruby 2.0 https://travis-ci.org/rails/rails/jobs/5811378#L589

@sferik
Copy link
Contributor Author

@sferik sferik commented on 08265a3 Mar 26, 2013

Choose a reason for hiding this comment

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

I ran tests on Ruby 1.9.3 before releasing but didn't think to run them again on Ruby 2.0.0? Have you bisected to determine specifically which commit is broke things?

@rafaelfranca
Copy link
Member

Choose a reason for hiding this comment

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

I'm running the tests right now. I'll bisect and send and report back later

@sferik
Copy link
Contributor Author

@sferik sferik commented on 08265a3 Mar 26, 2013

Choose a reason for hiding this comment

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

Thanks! If you can find the problem, I'm happy to push 0.18.1 later today.

@rafaelfranca
Copy link
Member

Choose a reason for hiding this comment

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

I could track down the commit that changed the behavior. https://github.com/thor/thor/commit/fb149e3dd8b71124d194cee81e5f5f1f788be6c5

But I think it is a Rails fault, it is defining a protected attr_reader to file_name, but is using in a template name that method.

I think that the methods used in template names should be public right?

@sferik
Copy link
Contributor Author

@sferik sferik commented on 08265a3 Mar 26, 2013

Choose a reason for hiding this comment

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

Any idea why they’re protected? Looks like that was added by @drogus in rails/rails@0134c5c.

@drogus
Copy link
Member

@drogus drogus commented on 08265a3 Mar 26, 2013

Choose a reason for hiding this comment

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

@sferik I don't think that there was any particular reason to make file_name protected, I was just following what was already there.

@rafaelfranca I'm not sure what's the difference in using the method in a template name and in the template itself, shouldn't both cases use NamedBase as a context?

@josevalim
Copy link
Contributor

Choose a reason for hiding this comment

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

The method is protected because if it is public, it will be run every time as a task, although for file_name, it is not really an issue.

In any case, I think the implementation in this commit fb149e3 is not good at all. I am skeptical all ruby implementations would have attr_* as the first line in the caller in there.

I think what Rails is doing is fine. The real bug is here: d02f275

When rendering files, Thor behaves like Sinatra. It works as if the file was embedded in the current Thor class. Therefore, restricting the filename to be just public methods is a silly limitation that doesn't work well with how Thor is meant to be used.

@rafaelfranca
Copy link
Member

Choose a reason for hiding this comment

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

👍 for not limiting the filename to be public methods.

@josevalim
Copy link
Contributor

Choose a reason for hiding this comment

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

I have opened a pull request that reverts these two regressions: #316

Please sign in to comment.