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

STunney/BobArnson: WIXFEAT:4239 #7

Merged
merged 2 commits into from Mar 28, 2014

Conversation

Projects
None yet
2 participants
@barnson
Member

barnson commented Mar 27, 2014

Add option to not extract the .msi when melting .wixpdbs. Don't leave temporary cabinet files behind (unless -notidy is in effect).

STunney/BobArnson: WIXFEAT:4239
Add option to not extract the .msi when melting .wixpdbs. Don't leave temporary cabinet files behind (unless -notidy is in effect).
@robmen

This comment has been minimized.

Show comment
Hide comment
@robmen

robmen Mar 28, 2014

Hmm. Historically, AppCommon.cs has only contained stuff shared across all the command-line tools. Nothing in the wix.dll actually referenced it (AFAIK). This changes that a bit. I'm kinda' okay with this change if you think this is the best place for WiX v3.x. However, in WiX v4.x it's on my list to completely revamp the way that we handle temporary files and moving them around...

robmen commented on src/tools/wix/AppCommon.cs in 30c70ba Mar 28, 2014

Hmm. Historically, AppCommon.cs has only contained stuff shared across all the command-line tools. Nothing in the wix.dll actually referenced it (AFAIK). This changes that a bit. I'm kinda' okay with this change if you think this is the best place for WiX v3.x. However, in WiX v4.x it's on my list to completely revamp the way that we handle temporary files and moving them around...

This comment has been minimized.

Show comment
Hide comment
@barnson

barnson Mar 28, 2014

Owner

Yuck, I hadn't realized it wasn't getting touched by wix.dll. This was the simplest change I came up with, given that Pdb.Save takes its own temp path. (The codedom temp support is wonky but at least it exists.) You have my full support in fixing it for reals in v4. :)

Owner

barnson replied Mar 28, 2014

Yuck, I hadn't realized it wasn't getting touched by wix.dll. This was the simplest change I came up with, given that Pdb.Save takes its own temp path. (The codedom temp support is wonky but at least it exists.) You have my full support in fixing it for reals in v4. :)

@robmen

This comment has been minimized.

Show comment
Hide comment
@robmen

robmen Mar 28, 2014

Maybe we could add AppCommon.DeleteDirectory() but have it pass through to Common.DeleteDirectory(). That may appear kinda' silly but it prevents changing all the other core classes (like Decompiler.cs and WixBinder.cs). Most importantly, it keeps the core classes from pointing "out" towards AppCommon, and instead points AppCommon "in" towards the core of wix.dll. That seems like more appropriate layering.

robmen commented on src/tools/melt/melt.cs in 30c70ba Mar 28, 2014

Maybe we could add AppCommon.DeleteDirectory() but have it pass through to Common.DeleteDirectory(). That may appear kinda' silly but it prevents changing all the other core classes (like Decompiler.cs and WixBinder.cs). Most importantly, it keeps the core classes from pointing "out" towards AppCommon, and instead points AppCommon "in" towards the core of wix.dll. That seems like more appropriate layering.

This comment has been minimized.

Show comment
Hide comment
@barnson

barnson Mar 28, 2014

Owner

And not the first time we've done that kind of thing. Will do.

Owner

barnson replied Mar 28, 2014

And not the first time we've done that kind of thing. Will do.

robmen added a commit that referenced this pull request Mar 28, 2014

Merge pull request #7 from barnson/feature4239
STunney/BobArnson: WIXFEAT:4239

@robmen robmen merged commit 6233430 into wixtoolset:develop Mar 28, 2014

@barnson barnson deleted the barnson:feature4239 branch Mar 30, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment