monterail / guidelines

[DEPRECATED] We are Ruby on Rails experts from Poland. Think hussars. Solid & winged. These are our guidelines.
71 stars 17 forks source link

Use active_attr instead of ActiveModel extensions directly #158

Closed sheerun closed 11 years ago

sheerun commented 11 years ago

It just look cleaner and have more features:

class Person
  include ActiveAttr::Model
end
teamon commented 11 years ago

include is a feature?

sheerun commented 11 years ago

No, read active_attr readme

chytreg commented 11 years ago

Nope, but you could read what the gem can do before you criticize!

:+1: for active_attr

teamon commented 11 years ago

IIRC we've already had a discussion about issues without description/arguments /cc @jandudulski

sheerun commented 11 years ago

It has an argument... Next time, I'll paste readme

sheerun commented 11 years ago

What I cannot say about your issue: https://github.com/monterail/guidelines/issues/157

teamon commented 11 years ago

If you treat #157 seriously...

sheerun commented 11 years ago

yes

jandudulski commented 11 years ago

:-1: for description

After short reading into docs I think you were ActiveAttr::BasicModel instead of ActiveAttr::Model in mind.

But for such a thing Rails 4 provides ActiveModel::Model which you can easily write in Rails 3.

For attributes I prefer Virtus. Any reasons for using ActiveAttr instead?

sheerun commented 11 years ago

No, I thought about ActiveAttr::Model. Virtus is not said to be compatible with active model. There are no validations, errors, block initialization, mass assignment, fields in attributes, and others. Active_attr has typecasting too. Any reasons to use Virtus instead?

jandudulski commented 11 years ago

Any reasons to use Virtus instead?

Do one thing well.

Virtus is not said to be compatible with active model

If you need to be compatible use ActiveModel::Model

There are no validations, errors

In the true OO validations shouldn't belong to model.

I understand your point, but AR is evil, we should try to look for ways how to escape from it instead of making PORO behaving like AM/AR.

sheerun commented 11 years ago

If you need to be compatible use ActiveModel::Model

What about Rails 3 Apps?

jandudulski commented 11 years ago

Put somewhere this snippet and just include.

sheerun commented 11 years ago

That's what active_attr is for....

sheerun commented 11 years ago

"If you want your model to be compatible with ActiveModel, include ActiveModel::Model in Rails 4, and include ActiveAttr::Model in Rails 3. If not, use Virtus instead"

sheerun commented 11 years ago

so what

sheerun commented 11 years ago

@jandudulski @teamon @Ostrzy @szajbus

teamon commented 11 years ago

:no_opinion:

chytreg commented 11 years ago

Agree with @sherun

Best regards Dariusz Gertych

2013/8/5 Tymon Tobolski notifications@github.com

:no_opinion:

— Reply to this email directly or view it on GitHubhttps://github.com/monterail/guidelines/issues/158#issuecomment-22103169 .

jandudulski commented 11 years ago

@sheerun please make a PR

sheerun commented 11 years ago

I can still make commits instead of PR :trollface: