<div dir="ltr">My two cents:<div><br></div><div style>All validators in a central, easy to find location.</div><div style>Validators apply to the application, and specifically the model.  So, +1 to either</div><div style><br>
</div><div style>app/validators (my preference)</div><div style><br></div><div style>or</div><div style><br></div><div style>app/models/validators</div><div style><br></div><div style>- Eric</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Feb 1, 2013 at 10:12 AM, Ivan Necas <span dir="ltr"><<a href="mailto:inecas@redhat.com" target="_blank">inecas@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<br>
----- Original Message -----<br>
> On Friday 01 of February 2013 12:53:09 Dmitri Dolguikh wrote:<br>
> > On 01/02/13 12:16 PM, Marek Hulan wrote:<br>
> > >> - A large number of validators have been moved into<br>
> > >> lib/validators. It<br>
> > >> could be argued that a few of those are not Katello-specific,<br>
> > >> but the<br>
> > >> bulk of them are. Any code that describes peculiarities of<br>
> > >> Katello<br>
> > >> domain belongs to app/model. Unless you are writing something<br>
> > >> non<br>
> > >> Katello-specific (ask yourself if the code could be packaged as<br>
> > >> a gem),<br>
> > >> it does not belong in lib/.<br>
> > ><br>
> > > Hi,<br>
> > ><br>
> > > this validators moving to one place was my commit. I see your<br>
> > > point and I<br>
> > > agree that some validators are Katello specific so lib/ is not<br>
> > > 100% right<br>
> > > place for them. However my main motivation was to get all<br>
> > > validators to<br>
> > > one place. Validators are not models so I think they should not<br>
> > > be in<br>
> > > app/models. Maybe we could move them to app/validators or if<br>
> > > you'd like<br>
> > > app/models/validators but second option would conflict with<br>
> > > namespacing<br>
> > > conventions... Also separating some validators to lib/ and some<br>
> > > to app/<br>
> > > seems to me like a useless obstacle for the future. No one wants<br>
> > > to think<br>
> > > about whether this validator is Katello specific or not.<br>
> > ><br>
> > > So to make long story short, it's not so important for me where<br>
> > > validators<br>
> > > will be placed but I'd really like to have them all in one<br>
> > > directory.<br>
> > ><br>
> > > Comments anyone?<br>
> ><br>
> > The model isn't something just entities that can be persisted in a<br>
> > database - the model is anything that contains "business-logic".<br>
> > That<br>
> > includes validators, services (things that perform operations on<br>
> > several<br>
> > model classes), etc. These all belong to models.<br>
> ><br>
> > Now, the source layout isn't as important in dynamic languages as<br>
> > it is<br>
> > with compiled languages, since we are not dealing with compilation<br>
> > units<br>
> > and are not trying to minimize compilation/build time. The source<br>
> > layout, however, can be used to simplify the discovery of model<br>
> > relations and functionality by borrowing projects using languages<br>
> > like<br>
> > java or C++: move the code closer to where it's being used and<br>
> > minimize<br>
> > the dependencies on the module.<br>
> ><br>
> > Here's my proposal:<br>
> >   - move shared, Katello-specific validators into<br>
> >   app/models/validators<br>
> >   - move unique (used by one model class) validators either in the<br>
> >   model<br>
> > classes themselves, or into correspondingly named modules in<br>
> > app/models/.<br>
> >   - leave non Katello-specific validators in lib/<br>
<br>
</div></div>Here's another point of view (that might bring some light as well):<br>
<br>
I started working on making our API documentation more DRYish. One<br>
of the things I would like to achieve is to get the information from<br>
validators (that bear very interesing documentation value). Few<br>
related findings:<br>
<br>
* We have duplicities in validations, compare:<br>
<br>
      ActivationKey#environment_not_library [1]<br>
<br>
   and<br>
<br>
      Changeset: validates_with Validators::NotInLibraryValidator [2]<br>
<br>
<br>
* We mix validating with method [1] vs. validating with class [2]. This might be worth<br>
  unifying<br>
<br>
* From the documentations perspecitve, there's no way you can extract anything from<br>
  the validations defined by method. In the [2] case, the validator appears in the<br>
  `Changeset.validators` and we can work with it further - The Apipie part is almost ready<br>
  for that.<br>
<br>
* If we extract the method validators into classes, its 15 more classes in the lib/validators<br>
  directory: might get messy, especially when some validators really are just applicable<br>
  for one class (such as Provider#constraint_redhat_update). Having them separated in a module<br>
  sounds reasonable, agreeing with dmitri<br>
<br>
* Having the validators in the `app/models` with all the models is quite messy too, and I agree<br>
  that it might lead in duplicate definitions, just becuase one doesn't notice something to<br>
  be used is already there. I don't<br>
  have much preferrences about `lib` vs `app/validators` vs `app/models/validators`.<br>
<br>
<br>
[1] - <a href="https://github.com/Katello/katello/blob/master/src/app/models/activation_key.rb#L64" target="_blank">https://github.com/Katello/katello/blob/master/src/app/models/activation_key.rb#L64</a><br>
[2] - <a href="https://github.com/Katello/katello/blob/master/src/app/models/changeset.rb#L43" target="_blank">https://github.com/Katello/katello/blob/master/src/app/models/changeset.rb#L43</a><br>
<div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> I don't agree. Benefits of having validators in one place I see:<br>
> + developers can see whether there is some existing/similar validator<br>
> already,<br>
> thus leads to more DRY code<br>
> + developers don't have to think whether some validator should be<br>
> considered<br>
> non Katello-specific (honestly - no one will ever use 10 lines in<br>
> form of our<br>
> validator in another application)<br>
> + developers know where to look at<br>
><br>
> And I don't see any real cons. I think we should not accept rules<br>
> just to be<br>
> 100% consistently organized without other benefits. I think we should<br>
> concentrate more on simplicity. The more rules we'll try to enforce<br>
> the harder<br>
> we'll be able to add new code. Let's try to keep it stupid simple -<br>
> it's<br>
> validator, move it into validators directory.<br>
><br>
> --<br>
> Marek<br>
><br>
> _______________________________________________<br>
> katello-devel mailing list<br>
> <a href="mailto:katello-devel@redhat.com">katello-devel@redhat.com</a><br>
> <a href="https://www.redhat.com/mailman/listinfo/katello-devel" target="_blank">https://www.redhat.com/mailman/listinfo/katello-devel</a><br>
><br>
<br>
_______________________________________________<br>
katello-devel mailing list<br>
<a href="mailto:katello-devel@redhat.com">katello-devel@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/katello-devel" target="_blank">https://www.redhat.com/mailman/listinfo/katello-devel</a><br>
</div></div></blockquote></div><br></div>