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

Create addRulesNamespace() in Validator. #89

Closed
wants to merge 1 commit into from
Closed

Conversation

augustohp
Copy link
Member

This will make easy to add new rules from namespaces we use.
We still have class_alias in order to do the same thing without
this new method.

This will make easy to add new rules from namespaces we use.
We still have `class_alias` in order to do the same thing without
this new method.
@augustohp
Copy link
Member Author

If people think this is usefull, we still need to change the README and do a release on this.

@alganet
Copy link
Member

alganet commented Oct 22, 2012

Couple of notes:

  • Exceptions have their namespace too, they're working fine? We need tests with check() and assert(), not only validate().
  • class_exists there is a major performance hit. class_alias is much more straightforward as well (no foreachs).

Personally, I don't like addRulesNamespace(). There are a plenty of ways of adding custom validators (addRule passing an instance and class_alias) that rely only on pure PHP/OOP. We never documented that though.

Also, we've been discussing this on another component, which seems to hold this responsibility better. @nickl- even did an implementation for it! Respect/Loader#3

@nickl-
Copy link
Member

nickl- commented Oct 25, 2012

Yes I was just going to say aren't we adding this to Loader.

quick loader status:
The Loader on Restler has been out in the wild for just over a month now, some issues have come back mainly due to the disabling of other loaders which obviously needs some more tlc, the issue IMO is not with the disabling but more so with the reintroduction process as they are all kept in queue again to be tried if discovery fails.

I know aff topic but while I'm at it and I have all your attention excuse me:

Respect/Cache: There's a board devoted on trello and I've implemented the the same design for aero's cache and also on Restler but I am thinking it must be a project on its own as we can use it in other places too.

Basically it's just a simple interface 2 methods which can handle all the use cases I think of.

seen(key, value) also does seen(key) and seen(map)
invalidate(key, time) also does invalidate(key), invalidate(key, true) and invalidate(true)

We won't be writing any cache solutions but if your implement this simple interface we can write wrappers for all the cache providers

see https://trello.com/c/Y6XcVZR6 for more detail.

Do you agree we make it a separate project to implement the wrappers and then Loader, Rest, probably Template could use in too... we just implement the interfaces and have a plug-able caching solution?

@augustohp augustohp closed this Nov 16, 2012
@nickl-
Copy link
Member

nickl- commented Jan 12, 2013

@alganet @augustohp @wesleyvicthor @henriquemoody @iannsp Guys I still haven't heard any feedback on my idea for Respect\Cache see #89-9767900 unless you have and I am not seeing the posts.

I am actually just looking for fresh eyes to see if they can spot any obvious shortcomings. The concept was employed on aero in python see cache.py

Ideally Respect\Cache would facilitate cache functionality as a wrapper to anything that implements the Interface which contracts the two methods seen and invalidate allowing you to write your implementation using a static collection or similar and at implementation time you can decide to extend it to use memcached/apc/file persist etc.

Do you foresee any immediate shortcomings, concerns something I may be missing? If you don't see any problems no comments are necessary and your agreement will be assumed.

Tx

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.

4 participants