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

Add XDTHelper #556

Merged
merged 26 commits into from Oct 27, 2014
Merged

Add XDTHelper #556

merged 26 commits into from Oct 27, 2014

Conversation

brianary
Copy link
Contributor

@brianary brianary commented Oct 7, 2014

Add support for invoking config file changes manually using idiomatic
FAKE syntax.

Add support for invoking config file changes manually using idiomatic
FAKE syntax.
@@ -0,0 +1,84 @@
[<AutoOpen>]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove auto open. We try to get rid of this.

@forki
Copy link
Member

forki commented Oct 7, 2014

How does this relate to https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/XMLHelper.fs#L182?

Can you please add some unit tests?

@brianary
Copy link
Contributor Author

brianary commented Oct 7, 2014

XMLHelper.XMLTransform uses XSLT, while XDTHelper uses Microsoft's XML
Document Transforms, a syntax optimized for making changes to config files.
(It was already referenced in the project, so there must be some awareness
of them.)
An argument could be made to move the functions into ConfigurationHelper,
though XDT aren't exclusively meant for config files, they can be a simple
alternative to full XSLT when only small changes are desired.
Moving the functions into XMLHelper could be done, but care would need to
be taken to prevent confusion between the two transformation types.

I should be able to add tests. What testing framework is that? Why are the
tests in C#?

Thanks for the quick reply.

On Mon, Oct 6, 2014 at 11:57 PM, Steffen Forkmann notifications@github.com
wrote:

How does this relate to
https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/XMLHelper.fs#L182?

Can you please add some unit tests?


Reply to this email directly or view it on GitHub
#556 (comment).

@forki
Copy link
Member

forki commented Oct 7, 2014

The testing framework is https://github.com/machine/machine.specifications

we use C# in order to dogfood the API's from C# and see how well they behave. In retrospect this wasn't the best decision, because actually we don't care about C# usage of FAKE's API.

Add support for invoking config file changes manually using idiomatic
FAKE syntax.
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
@brianary
Copy link
Contributor Author

Would adding a new unit test project that uses Unquote
https://code.google.com/p/unquote/ be acceptable?

On Tue, Oct 7, 2014 at 7:38 AM, Steffen Forkmann notifications@github.com
wrote:

The testing framework is https://github.com/machine/machine.specifications

we use C# in order to dogfood the API's from C# and see how well they
behave. In retrospect this wasn't the best decision, because actually we
don't care about C# usage of FAKE's API.


Reply to this email directly or view it on GitHub
#556 (comment).

@forki
Copy link
Member

forki commented Oct 11, 2014

Why not a small NUnit project?

@brianary
Copy link
Contributor Author

Wouldn't that be just moving from one C# testing framework to another?

FSUnit could be an option, but it sounds like that is an F# wrapper over an
idiomatically C# framework.
Unquote purports (though I haven't yet tried it) to be more natively F#.

SpecFlow could be another option, but it mostly just takes advantage of
F#'s ability to use regular expressions as function names.

I can learn machine.specifications if you'd prefer to stick with that. I
just got the impression that the rationale for that choice was no longer
compelling.

Let me know which test framework you'd prefer, and I'll get started. :)

Thanks!

On Sat, Oct 11, 2014 at 3:49 AM, Steffen Forkmann notifications@github.com
wrote:

Why not a small NUnit project?


Reply to this email directly or view it on GitHub
#556 (comment).

brianary and others added 18 commits October 17, 2014 02:37
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
- When the target is -lt or --listTargets the command line will show the
  list of targets with dependencies
- The option -lt or --listTargets show the list of targets.
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
Add support for invoking config file changes manually using idiomatic
FAKE syntax.
@brianary
Copy link
Contributor Author

Simple Machine.Specs tests added to support XDTHelper.

@brianary
Copy link
Contributor Author

Should be ready now.

@forki forki merged commit a90f397 into fsprojects:master Oct 27, 2014
@forki
Copy link
Member

forki commented Oct 27, 2014

thanks I merged it.

Could you please rebase the next time? Would make it much easier for me.

xdt

@forki
Copy link
Member

forki commented Oct 27, 2014

mmh. I didn't see this before but it doesn't build on mono: https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?

@brianary
Copy link
Contributor Author

I don't really have a mono dev environment on my Linux box, but I can try
it when I get home.

It looks like the Microsoft.Web.XmlTransform.dll (that was already included
with the project) isn't working in mono, maybe?
On Mon, Oct 27, 2014 at 10:19 AM, Steffen Forkmann <notifications@github.com

wrote:

mmh. I didn't see this before but it doesn't build on mono:
https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?


Reply to this email directly or view it on GitHub
#556 (comment).

@brianary
Copy link
Contributor Author

Thinking about it, I wonder if it's my use of backslashes in the test
paths. I'll remediate that first.

On Mon, Oct 27, 2014 at 10:50 AM, Brian Lalonde brianary@gmail.com wrote:

I don't really have a mono dev environment on my Linux box, but I can try
it when I get home.

It looks like the Microsoft.Web.XmlTransform.dll (that was already
included with the project) isn't working in mono, maybe?
On Mon, Oct 27, 2014 at 10:19 AM, Steffen Forkmann <
notifications@github.com> wrote:

mmh. I didn't see this before but it doesn't build on mono:
https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?


Reply to this email directly or view it on GitHub
#556 (comment).

@brianary
Copy link
Contributor Author

Before ./build.sh would work, I had to mozroots --import --sync, as
mentioned in paket issue 271
fsprojects/Paket#271. (Maybe worth adding to
the script?)

After that, it built fine on my Ubuntu box, so I'm not yet able to
reproduce the failure. :/

On Mon, Oct 27, 2014 at 4:12 PM, Brian Lalonde brianary@gmail.com wrote:

Thinking about it, I wonder if it's my use of backslashes in the test
paths. I'll remediate that first.

On Mon, Oct 27, 2014 at 10:50 AM, Brian Lalonde brianary@gmail.com
wrote:

I don't really have a mono dev environment on my Linux box, but I can try
it when I get home.

It looks like the Microsoft.Web.XmlTransform.dll (that was already
included with the project) isn't working in mono, maybe?
On Mon, Oct 27, 2014 at 10:19 AM, Steffen Forkmann <
notifications@github.com> wrote:

mmh. I didn't see this before but it doesn't build on mono:
https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?


Reply to this email directly or view it on GitHub
#556 (comment).

@brianary
Copy link
Contributor Author

Scratch that, I can reproduce it.

It looks like it a problem with the Microsoft.Web.XmlTransform.dll on linux.
The MonoDevelop NuGet addin has run into this
mrward/monodevelop-nuget-addin#16, as has
Xamarin https://bugzilla.xamarin.com/show_bug.cgi?id=19426.

Not really sure how to mitigate this. What else is this library being used
for?

On Mon, Oct 27, 2014 at 8:49 PM, Brian Lalonde brianary@gmail.com wrote:

Before ./build.sh would work, I had to mozroots --import --sync, as
mentioned in paket issue 271
fsprojects/Paket#271. (Maybe worth adding to
the script?)

After that, it built fine on my Ubuntu box, so I'm not yet able to
reproduce the failure. :/

On Mon, Oct 27, 2014 at 4:12 PM, Brian Lalonde brianary@gmail.com wrote:

Thinking about it, I wonder if it's my use of backslashes in the test
paths. I'll remediate that first.

On Mon, Oct 27, 2014 at 10:50 AM, Brian Lalonde brianary@gmail.com
wrote:

I don't really have a mono dev environment on my Linux box, but I can
try it when I get home.

It looks like the Microsoft.Web.XmlTransform.dll (that was already
included with the project) isn't working in mono, maybe?
On Mon, Oct 27, 2014 at 10:19 AM, Steffen Forkmann <
notifications@github.com> wrote:

mmh. I didn't see this before but it doesn't build on mono:
https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?


Reply to this email directly or view it on GitHub
#556 (comment).

@brianary
Copy link
Contributor Author

brianary commented Nov 1, 2014

I've updated the tests to use the correct path separator, as I was using
locally.

In re-reading the Xamarin bug
https://bugzilla.xamarin.com/show_bug.cgi?id=19426, it appears the reason
XDT is incompatible with Mono is that the
System.Xml.XmlAttribute.set_Prefix behavior does not match .NET (.NET
accepts a null and sets the prefix to an empty string, while Mono throws an
exception).

It also sounds like there may be licensing issues with distributing the
original Microsoft DLL, as mentioned in the MonoDevelop NuGet bug
mrward/monodevelop-nuget-addin#16. The
open-source version also mentioned claims not to send the null value that
Mono objects to, although Mono's behavior itself should probably be matched
to .NET for parity.

I'm not finding usage of Microsoft.Web.XmlTransform prior to XDTHelper,
though it was already included and appears in fsproj and app.config files.
I'm just wondering if there's more that's not working as expected that
maybe we just don't know about.

On Mon, Oct 27, 2014 at 10:09 PM, Brian Lalonde brianary@gmail.com wrote:

Scratch that, I can reproduce it.

It looks like it a problem with the Microsoft.Web.XmlTransform.dll on
linux.
The MonoDevelop NuGet addin has run into this
mrward/monodevelop-nuget-addin#16, as has
Xamarin https://bugzilla.xamarin.com/show_bug.cgi?id=19426.

Not really sure how to mitigate this. What else is this library being used
for?

On Mon, Oct 27, 2014 at 8:49 PM, Brian Lalonde brianary@gmail.com wrote:

Before ./build.sh would work, I had to mozroots --import --sync, as
mentioned in paket issue 271
fsprojects/Paket#271. (Maybe worth adding to
the script?)

After that, it built fine on my Ubuntu box, so I'm not yet able to
reproduce the failure. :/

On Mon, Oct 27, 2014 at 4:12 PM, Brian Lalonde brianary@gmail.com
wrote:

Thinking about it, I wonder if it's my use of backslashes in the test
paths. I'll remediate that first.

On Mon, Oct 27, 2014 at 10:50 AM, Brian Lalonde brianary@gmail.com
wrote:

I don't really have a mono dev environment on my Linux box, but I can
try it when I get home.

It looks like the Microsoft.Web.XmlTransform.dll (that was already
included with the project) isn't working in mono, maybe?
On Mon, Oct 27, 2014 at 10:19 AM, Steffen Forkmann <
notifications@github.com> wrote:

mmh. I didn't see this before but it doesn't build on mono:
https://travis-ci.org/fsharp/FAKE/builds/39173705

could please try to check?


Reply to this email directly or view it on GitHub
#556 (comment).

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.

None yet

3 participants