tractorcow / silverstripe-dynamiccache

Simple on the fly caching of dynamic content for Silverstripe
39 stars 27 forks source link

Ss4 upgrade #79

Open priyashantha opened 6 years ago

priyashantha commented 6 years ago

Related to the issue https://github.com/tractorcow/silverstripe-dynamiccache/issues/69

undefinedplayer commented 6 years ago

@tractorcow It'll be great if someone can review this. Appreciate.

mkrauser commented 6 years ago

Hey @priyashantha,

I've also worked on this one and I've got a few suggestions:

You can take a look at my ss4-branch for the first two points: https://github.com/mkrauser/silverstripe-dynamiccache/tree/ss4 Maybe you can re-use some of my work.

Thanks you for your effort!

tractorcow commented 5 years ago

@mkrauser would you like me to just merge your version as the ss4 version? I'm of a mind to let you two decide since you probably have more context over the current upgrade than I do. :)

priyashantha commented 5 years ago

Thanks @mkrauser for your comments on my update. I'll have a look at them. But I couldn't have a chance to check your fork yet.

So @tractorcow Please merge that if there is no issue there.

tractorcow commented 5 years ago

I've created https://github.com/tractorcow/silverstripe-dynamiccache/pull/81 as a comparison.

What version do people prefer I merge?