search

plexus / yaks

Ruby library for building hypermedia APIs
http://rubygems.org/gems/yaks
MIT License
236 stars 26 forks source link

instance_eval if option in attributes #117

Closed hallgren closed 8 years ago

hallgren commented 8 years ago

A new PR for #116 fixing the commit messages.

groyoh commented 8 years ago

It would have been better to squash the commits instead of opening a new PR. Could you amend the commit and add a better description of the change?

For example:

Add specs for Yaks::Mapper::Attribute

The specs now test that the "if" option of Yaks::Mapper::Attribute accepts procs and lambdas.
hallgren commented 8 years ago

To do that I had to rewrite the history of an already pushed branch. Thats why I created the new PR.

groyoh commented 8 years ago

It's usually ok to rewrite the history (rebase) of your own branch (even if it's pushed) as long as nobody else is working on it and it isn't merged. In the case of PRs, it's often recommended to squash before merging to keep history clean of WIP commits. And that's usually done by rebasing and force pushing ;) This also avoid creating too many PRs.

hallgren commented 8 years ago

@groyoh I made the adjustment you wanted. Is it ok?

groyoh commented 8 years ago

The PR looks great :+1: Just need to squash it as described in https://github.com/plexus/yaks/pull/117#issuecomment-199753388

hallgren commented 8 years ago

@groyoh squashed :)

groyoh commented 8 years ago

Thanks for rebasing. As requested many times by @plexus and I, could you please provide a better commit message? instance_eval if option isn't actually what the commit does and is not a proper commit message. Check the example I gave you at https://github.com/plexus/yaks/pull/117#issuecomment-199753388

hallgren commented 8 years ago

@groyoh I have now changed the commit message to what I think it does, thanks for your comment.

groyoh commented 8 years ago

Thanks for the PR :+1: I'm merging it.

hallgren commented 8 years ago

@groyoh Are there any plan in publish i public release to rubygems?

groyoh commented 8 years ago

According to @plexus comment, he will probably put out a new release. :wink: @plexus could you confirm that?

plexus commented 8 years ago

Released as v0.12.0, thanks for all the help!

hallgren commented 8 years ago

nice! thanks