search

aspnet / MusicStore

[Archived] MusicStore test application that uses ASP.NET/EF Core. Project moved to https://github.com/aspnet/AspNetCore
1.3k stars 878 forks source link

added missing reference to System.Data.Entity which causes build to fail. #540

Closed redwards510 closed 9 years ago

redwards510 commented 9 years ago

The line UseGlobalApplicationHostFile just added itself after I built, so I assume that's ok.

dnfclas commented 9 years ago

Hi @redwards510, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

Eilon commented 9 years ago

Does this happen in the dev branch? That's the most current branch. The master branch here is for beta7 so I want to make sure there isn't a mismatch here.

redwards510 commented 9 years ago

I haven't tried the dev branch. I just cloned the repository so I could try playing around with this project and it fails to build because of the missing reference.

Eilon commented 9 years ago

@kichalla canyou take a look to see if master is broken?

kichalla commented 9 years ago

@Eilon based on the PR change, its a change in MvcMusicStore.csproj, so our builds would not pick it up?

ajcvickers commented 9 years ago

@Eilon Are you sure this is something we want to do? System.Data.Entity is the namespace from legacy versions of EF. Unless something has changed I don't think we want MusicStore to reference this.

Eilon commented 9 years ago

@ajcvickers it's the MvcMusicStore project (the original one using .NET 4.x) that was using EF, so I think in principal that's fine. However, we just deleted MvcMusicStore from the dev branch so I'm not sure it's worth fixing something that we just deleted. All we kept in dev was the ASP.NET 5 / EF 7 version of MusicStore.

Nevertheless, I'm curious how we didn't notice that it was broken...

But given all that, I suspect it's fine to close this PR.

ajcvickers commented 9 years ago

@Eilon Okay, that makes sense.

Eilon commented 9 years ago

Closing this because in 1 week this won't matter (we deleted the affected project).