Skip to content

Conversation

@misoeli
Copy link
Member

@misoeli misoeli commented Jul 18, 2022

Allow for overriding the Xrm namespace, providing a custom one if needed.

Instead of wrapping Xrm in another namespace level (which would require an additional namespace declaration in both xrm.d.ts AND all version-specific files, simply renaming the namespace keeps the needed changes down.

Still not sure it works. It looked as if VS was acting up post generation, but I would prefer a re-review.

@misoeli
Copy link
Member Author

misoeli commented Jul 18, 2022

Fixes #181 .

@misoeli
Copy link
Member Author

misoeli commented Jul 19, 2022

@skovlund Looks OK to me after retesting. Most likely just a case of old dependencies. Would like you to have a look though.

@majblackburn
Copy link

pulling and attempting to compile (although I'm not an F# dev). Thanks SO MUCH for this -- we were a bit stuck on our attempt to bring TDD into a project with XrmDT!

@majblackburn
Copy link

This has resolved our issues with the conflict between @types/xrm and XrmDT. I'd like to suggest "XrmDT" instead of "xrmns" as a default, but I'm in no way complaining! Thanks again!

@skovlund
Copy link
Contributor

skovlund commented Aug 1, 2022

@misoeli I've looked through the changes and it looks fine. However, I've not tested and therefore have not verified that we have caught every instance of "Xrm".

@majblackburn I'm not entirely sure I understand your suggestion. The namespace still defaults to "Xrm" (for backward compatibility) and not to either "XrmDT" nor "xrmns" (by the way, we usually abbreviate the tool "XDT"). However, "XRMNS" is used internally in code as placeholder text before replacing it with the custom namespace name. If you are referring to the argument "xrmNamespace" given the alternative command "xrmNs" I think this might be better abbreviated to simply "ns".

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.

4 participants