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

Added new callbacks to action. #83

Merged
merged 1 commit into from
Jan 30, 2015
Merged

Added new callbacks to action. #83

merged 1 commit into from
Jan 30, 2015

Conversation

AlfonsoUceda
Copy link
Contributor

Added prepend_before, prepend_after, append_before and append_after.

Closes #76

@jodosha jodosha self-assigned this Jan 29, 2015
@jodosha jodosha added this to the v0.3.2 milestone Jan 29, 2015
@jodosha
Copy link
Member

jodosha commented Jan 29, 2015

@AlfonsoUceda Thank you! This looks great for me. 👍

@AlfonsoUceda
Copy link
Contributor Author

I didn't introduce more tests, I think is enough

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 99.11% when pulling 3320ea0 on AlfonsoUceda:master into 8c299e8 on lotus:master.

@jodosha
Copy link
Member

jodosha commented Jan 29, 2015

@joneslee85 and @weppos Do you mind to review this?

@weppos
Copy link
Member

weppos commented Jan 29, 2015

One concern. I wonder if the primary method should be append_before and the alias before, rather than the opposite, for consistency with prepend_before.

@jodosha
Copy link
Member

jodosha commented Jan 29, 2015

@weppos That was mine too. Now we're two people concerned with it. 👍 for this change.
If there isn't any stopping point, this should be released on tomorrow as v0.3.2.

@AlfonsoUceda
Copy link
Contributor Author

ok @jodosha and @weppos I'm going to do that change ;) 👍 for the change

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 99.11% when pulling 0f751e6 on AlfonsoUceda:master into 8c299e8 on lotus:master.

@runlevel5
Copy link
Member

me like, me 👍

@jodosha jodosha merged commit 0f751e6 into hanami:master Jan 30, 2015
jodosha added a commit that referenced this pull request Jan 30, 2015
@jodosha
Copy link
Member

jodosha commented Jan 30, 2015

@AlfonsoUceda I've merged this, thank you! 👍

@weppos
Copy link
Member

weppos commented Jan 30, 2015

Thanks @AlfonsoUceda for the work. 👏

@AlfonsoUceda
Copy link
Contributor Author

you welcome mates ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to configure a filter in a specific position
5 participants