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

Loosen System.Web Stranglehold #315

Open
AlexCuse opened this issue Jun 26, 2015 · 11 comments
Open

Loosen System.Web Stranglehold #315

AlexCuse opened this issue Jun 26, 2015 · 11 comments
Labels

Comments

@AlexCuse
Copy link
Collaborator

Hide everything that depends on System.Web behind an interface, allow user to specify alternate implementations.

Probably a good change for v1.0, take the opportunity to overhaul configuration API to make it more discoverable.

BundleCache / RawContentCache and DebugStatusReader are the two remaining that come to mind, though I'd imagine there are others.

It'd be nice to put implementations of these interfaces into a new project/package for nancy users, though I am not sure I have the expertise with nancy to figure this stuff out (CacheDependency is an especially glaring need).

@AlexCuse AlexCuse added the v1.0 label Jun 26, 2015
@AlexCuse AlexCuse mentioned this issue Jun 26, 2015
11 tasks
@AlexCuse
Copy link
Collaborator Author

I think a new assembly will be needed for the aspnet specific bits but we can include in the current squishit package to minimize disruption. A .nancy package could then include alternate implementations, along with the framework assembly.

Now that we're on .net 4 can make these satellite assemblies register themselves with the framework (like the pre processors do).

@Worthaboutapig
Copy link

Is the SquishIt.Mvc not the appropriate place to move the ASP.NET-specific stuff? Or do you support WebForms?
Do we not need a SquishIt.Nancy assembly too?

Are you happy for me to pull out the necessary interfaces, or do you want to set up the initial work?

@Worthaboutapig
Copy link

I'm put something together as a start, I'll create a PR for you to have a look. Try and have it done in the next couple of days, but might not have much time tomorrow ;)

@AlexCuse
Copy link
Collaborator Author

MVC package is just razor helper stuff, I've never actually used it. I'm thinking a SquishIt.AspNet assembly that gets included with current package will be the way to go. Then if you get nancy implementations together we can put out SquishIt.Nancy and include a pointer in the release notes for any nancy users.

Yeah if you want to pull out the interfaces thats fine. Just create some folder in the Framework project for now and we can get it moved out to a new assembly when ready.

Definitely thinking this should be included in 1.0 release since its a pretty major change.

You can fork from here: https://github.com/AlexCuse/SquishIt/tree/feature/v1.0

@Worthaboutapig
Copy link

Thanks. Now have to figure out the best way to fork it, as I can't. I've already forked the jetheredge/SquishIt, so GitHub won't allow me to refork the AlexCuse version, it just points me back to the jetheredge version. And that doesn't have the branches. Could delete my original fork, but don't really want to, or setup another account, but don't want to do that either :S.

(see http://stackoverflow.com/questions/12338240/how-can-i-fork-the-original-repo-when-ive-already-forked-a-different-fork?lq=1)

Any suggestions?

edit: Actually, is there a specific reason the 1.0 branch is not in the parent repository? Then I wouldn't have a problem ;)

@Worthaboutapig
Copy link

Ok, I've managed to manually create a fork... however, after switching to your feature/v1.0, I have 40 failed tests. Is this expected?

They're all related to the expected minified script being function product(n,t){return n*t}function sum(n,t){return n+t}\n, whereas it's coming out as function product(c,d){return c*d}function sum(c,d){return c+d}\n e.g. in SquishIt.Tests.JavaScriptBundleTests.CanBundleArbitraryContentsInRelease

@AlexCuse
Copy link
Collaborator Author

AlexCuse commented Jan 3, 2016

Oh man sorry about that - just committed a change flipping the default minifiers back to ajaxmin. I got cold feet about that change in light of #323.

@AlexCuse
Copy link
Collaborator Author

AlexCuse commented Jan 7, 2016

re self registering assemblies (when SquishIt.AspNet gets split out):

AlexCuse@4c4b905

@Worthaboutapig
Copy link

@AlexCuse Could you take a look at the PR I sent you: Worthaboutapig/AC-SquishIt#2

@AlexCuse
Copy link
Collaborator Author

I don't understand that PR - what does it have to do with extracting dependencies on System.Web? Are there unpushed commits?

I am not sure I want to make this change, because the sass project seems to be working for some people and it might break it. I have been watching libsass.net and there seems to be movement in making it "AnyCPU" ready.

@AlexCuse
Copy link
Collaborator Author

Oh wait I see, its PR 1 in your repo not 2.

I never got a notification for this - it looks like you submitted the PR against your repo's feature/v1.0 branch instead of upstream. It does look more or less like what I had in mind though.

Don't worry too much about coding style, though I appreciate your efforts. The project has been around for a while, and its had contributions from a lot of people. I can never quite get a satisfactory result from resharper's "solution wide" cleanup. I especially appreciate your inclusion of xml comments, as I am on the verge of starting to include them in nuget packages for http://dotnetapis.com. Now that this is getting close I will probably wait until we have the new nuget packages defined so I can include them everywhere at once.

Let me know how everything goes, and if I can help with anything. I think you can get my email from the git logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants