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

Prevent dotnet template engine to parse Paket.Restore.targets #1069

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

MangelMaxime
Copy link
Member

@alfonsogarciacaro Careful when you run paket.exe update/install perhaps this will override this changes.

@forki could probably confirm/infirm the behaviour of paket on this file. :)

@alfonsogarciacaro
Copy link
Member

Sorry, I don't understand exactly why this change is needed, could you explain more? I usually use the template as a normal project to test everything works before publishing a new version, so I'm afraid I or another contributor at one point will run paket update and forget about this. Also, if this is something necessary for Paket, shouldn't it be added directly to Paket targets as it will affect all templates using Paket?

@forki
Copy link
Collaborator

forki commented Jul 15, 2017 via email

@alfonsogarciacaro
Copy link
Member

@forki I see, but I think my points above are still valid. Sorry, but I'm not crazy supporting a non-stable version of dotnet SDK by adding patches and maintenance chores... specially not in Summer 😉 😎 🌞 🌻

I'd rather change the README to say dotnet SDK 2.0 is not supported yet or add the recommendation to run paket install after running the template.

@forki
Copy link
Collaborator

forki commented Jul 15, 2017

Just take a look at recent fable gitter. Everyone runs into this. Everyone. So we need to do something...

@alfonsogarciacaro
Copy link
Member

These are the scenarios I foresee:

  • A couple of weeks later I make a fix to Fable and want to update the template. I run paket update to test the template works with latest Fable, this overwrites the change from this PR, and the chances that I forget to undo that while I'm in a hurry and focusing on another thing are really high.
  • Even if I remember to undo the change for Paket.restore.targets, what if Paket is actually changing something to fix another issue? Do I have to check the diffs every time and move the <!--/-:cnd:noEmit--> comments manually?

Because of this, I would prefer an alternative solution:

  • State in the README that Fable explicitly requires dotnet SDK 1.0.4; or
  • Add the recommendation to run paket install if you're using dotnet SDK 2 (actually I think with dotnet SDK 2 you can run operations automatically).
  • If this affects all templates, why not add it to Paket targets directly? It's just a comment and shouldn't affect normal Paket users.

@forki
Copy link
Collaborator

forki commented Jul 15, 2017

Yes adding it to paket itself is probably best.

@alfonsogarciacaro alfonsogarciacaro merged commit c61e830 into master Jul 25, 2017
@alfonsogarciacaro alfonsogarciacaro deleted the fix_template branch November 30, 2017 20:36
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.

3 participants