Skip to content

Conversation

@lookyman
Copy link

Ok, this PR might be a little over the top, but I started playing with it and ended up with this. It's still not finished, I need to add more tests for Extension and especially Route, do some more prettying, clean up, and stuff like that. But I want to hear your opinions.

Summary of changes

  • Extension renamed to DI\Extension.
  • Route renamed to Application\Route.
  • Macros renamed to Latte\Macros.
  • Added Application\Response, handles actual sending.
  • Added Rule, encapsulates validation rules.
  • Added Helpers, for some misc functions.
  • Cleaned code, added annotations where necessary.
  • Config is validated in Extension.
  • Routes are prepended to the router. (thanks @Kdyby)
  • Parameter "algorithm" renamed to "flags", to be consistent with Nette\Utils\Image.
  • PHP compatibility extended backwards down to PHP 5.3.3.
  • Added tests for everything I could think of.
  • Moved some stuff around. A lot.
  • Travis CI ready.
  • You are not forced to register a provider.

@vojtech-dobes
Copy link
Collaborator

Hi, thanks for all great work! Please let me know when you are satisfied with it, then I will walk through it and comment (some things I like, some don't).

@lookyman
Copy link
Author

Well, basically I think it's ready, except for tests. Ofc those could show something is broken, but the general shape should not change much even if they do..

@vojtech-dobes vojtech-dobes mentioned this pull request Sep 18, 2014
@lookyman
Copy link
Author

Ok now I think I'm finished. There's always more that could be done, but that can be left to future PRs. Let me know after you've read through it..

@f3l1x
Copy link

f3l1x commented Sep 23, 2014

I like

  • namespaces
  • image response
  • rule
  • config validation

I don't think is needed

  • route prependig
  • php 5.3

@lookyman
Copy link
Author

I can get rid of route prepending, or possibly add a configuration option to prepend instead of default append. Basically the only reason I added it in the first place was to work around a problem I had in one of my projects, and this was the easiest way (modify the thing I am trying to add instead of touching the stuff I have already written).

I'll wait for @vojtech-dobes, I'm sure he'll have a lot to add.

@Koricz
Copy link

Koricz commented Jul 19, 2015

I really looking forward when this PR will be merged, but I think there still should be an option passing all parameters from template link to Generator and Provider(s)...

https://github.com/lookyman/nette-webimages/blob/fc9e83e54a7b9651bcf76b18f158c0d6838e8e6e/src/Application/Route.php#L57

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