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

Fix transform condition #36

Conversation

luuksommers
Copy link
Contributor

I think the condition is wrong, I've tried to go on without any Web.Helix.config and setting the IncludeHelixWebConfigTransformInPackage to false, but I would always get the message No transforms were supplied at packages\RichardSzalay.Helix.Publishing.WebRoot.1.4.1\build\Helix.Publishing.Plugins\MergeHelixModuleWebConfigTransforms.targets(55,5). I think this is because the $(HelixModuleWebConfigTransforms) always has a value, even though there are no files found.

@richardszalay
Copy link
Owner

Ah damn. I guess my fix for transforms introduced an issue for when there are no transforms. There's no test case for that.

If you just want to get your build working again, jump back to 4.0.0.

If you'd like to supply a PR, just add support in the Feature/Foundation CSPROJ so that the Web.Helix.config <Content> element has a condition Condition="'$(ExcludeHelixTransforms)' != 'true'". Then add a test for "applies default transform when no helix transforms are available"

@richardszalay
Copy link
Owner

(just realised this is a PR)

@richardszalay
Copy link
Owner

I'm going to reject this PR as the bug is unrelated to IncludeHelixWebConfigTransformInPackage. Your change would only ever apply Web.Helix.config transforms when the user wants to include the merged transform file (as well as the transformed Web.config) in the package. If you want to disable the "merge helix transforms" feature, you should set MergeHelixModuleWebConfigTransforms to false.

Having said that, the handling of a lack of transforms is indeed broken so I'm creating some tests and fixing that behaviour now.

@richardszalay
Copy link
Owner

@luuksommers Can you check1.4.2-beta1 and see if it resolves the issue for you? If so, I'll publish it as a full release.

@luuksommers
Copy link
Contributor Author

luuksommers commented Nov 23, 2018 via email

@richardszalay
Copy link
Owner

Released as 1.4.3 (1.4.2 was published in error)

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.

2 participants