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

Allow symbols to be passed to the 'on' option for single events #264

Merged
merged 1 commit into from
Aug 30, 2013

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Aug 29, 2013

Generally in rails, when an option can be passed with an array of symbols, a symbol is allowed in place of a single element array.

@ghost ghost assigned batter Aug 29, 2013
@batter batter merged commit 28d4a42 into paper-trail-gem:master Aug 30, 2013
@batter
Copy link
Collaborator

batter commented Aug 30, 2013

@sgrif - Thanks for the pull request, completely agree with the concept, actually a little surprised this wasn't already a feature, considering we already do it with other options like :ignore, :skip, :only.

The test was failing though due to a typo (instance_evail should be instance_eval), and the initializer portion needed to be wrapped in a setup block; not sure if you tried running the test locally first?

I've merged in the pull and fixed the test so it passes. This also brought to my attention that the new rake task I had implemented to run both test suites wasn't exiting with an error because I was using system to invoke the tasks instead of a task pointing to the other two tasks, and as such Travis-CI was not recognizing failing tests. This gave me an opportunity to notice and fix that issue as well, so cheers for that! (see 2d1ee54)

@sgrif
Copy link
Contributor Author

sgrif commented Aug 30, 2013

I could have sworn I did (I actually even remember fixing that typo.)
Either way, glad to help!
On Aug 30, 2013 1:17 PM, "Ben Atkins" notifications@github.com wrote:

@sgrif https://github.com/sgrif - Thanks for the pull request,
completely agree with the concept, actually a little surprised this wasn't
already a feature, considering we already do it with other options like
:ignore, :skip, :only.

The test was failing though due to a typo (instance_evail should be
instance_eval), and the initializer portion needed to be wrapped in a
setup block; not sure if you tried running the test locally first?

I've merged in the pull and fixed the test so it passes. This also brought
to my attention that the new rake task I had implemented to run both test
suites wasn't exiting with an error because I was using system to invoke
the tasks instead of a task pointing to the other two tasks, and as such
Travis-CI was not recognizing failing tests. This gave me an opportunity to
notice and fix that issue as well, so cheers for that!


Reply to this email directly or view it on GitHubhttps://github.com//pull/264#issuecomment-23583419
.

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.

2 participants