Outlook does not display a logo in the header

[snd3r]

Great project guys, thank you very much for it! I really like the coding style. You are great.

I have one improvement: by default Outlook does not display a logo in the header

"To help protect your privacy, Outlook prevented automatic download of some pictures in this message"

I solved this problem by adding images inlining: https://github.com/snd3r/PSSharedGoods/commit/95d801715dbc00aa0b91ac0f59c5f5fd6509e7f8 https://github.com/snd3r/PSWinReporting/commit/8bc74b73a63c531c8892495d483761ea9a0931e3

Can I send a pull request?

[PrzemyslawKlys]

Hi,

First of all - Always. Any PR's are allowed (for any projects I run) and even more are welcomed with open arms :-)

Now there is only few things I would like to keep:

• Don't disable old feature but keep the old one and add some configuration for it (in "starting hashtable"). That way we cover old way and new way. There is a reason why I did this as your way is not always displaying correctly in some scenarios either. I'm happy to have a choice and to give a choice. Even thou I may not use this one, you will so it's great to have it.
• For that new feature add configuration check (there is a function inside that requires hashtable key to exists). This makes sure that if user upgrades he gets terminated on checking the configuration and therefore he can go thru changes that were implemented to understand new features. I do this to not surprise users.

I would also need an explanation (it's a legit question that I would like to understand):

• benefit $PSBoundParameters.ContainsKey vs $InlineAttachments -eq $nul ? I have seen this few times but have not bothered to go deep to find the reasoning for it over theway I use it.
• what does alternateview gives?
• what is the benefit of Get-ContentType inside a function? I do keep each function separate even thou it may be used only by Send-Email only. But maybe there is some performance impact? If no I would prefer to have it as a separate function one can use for other stuff if need arises.

Good enough?

[snd3r]

Don't disable old feature but keep the old one and add some configuration for it (in "starting hashtable"). That way we cover old way and new way.

Got it.

benefit $PSBoundParameters.ContainsKey vs $InlineAttachments -eq $nul ?

Benefit in explicit simplicity. If we use $InlineAttachments -eq $null, we extend the scope to the whole script and may be affected by side effects. If we use $PSBoundParameters.ContainsKey, we reduce the scope, declare checking input parameters explicitly and we no longer need defaults args value. Imo this code looks cleaner.

what is the benefit of Get-ContentType inside a function?

Benefit - decrease the scope, a decrease cognitive load. It seemed to me not right to tear this small helper function from the context, I do not think that it will be useful elsewhere. If you think it's right to move it to separate file, let's do it. The number of mime types can be increased, maybe then its separation be more appropriate.

[PrzemyslawKlys]

Great. Thank you for explanations. Please extract function outside to Objects folder. I'm sure it will come in handy for something else ;-) I'm slowly building up PSSharedGoods to be my go to module for multiple functions. I also fix and clean them up when I have time as I know there are dragons there ;-)

Feel free to submit PR when ready ;-) And I'm very open to seeing more work from you!

[snd3r]

@PrzemyslawKlys done: https://github.com/EvotecIT/PSSharedGoods/pull/1

[PrzemyslawKlys]

Great. Merged but I guess you need to fix still commit on PSWinReporting?

[snd3r]

@PrzemyslawKlys sorry, I do not understand what I need to do (fix). This is my first PR, if you tell me what to do, I will be very grateful! What change do I need to add to this PR? https://github.com/EvotecIT/PSWinReporting/pull/10

[PrzemyslawKlys]

I think you fixed it now. Before you overwrote the height/width settings as those are not needed for inline I guess. It's still needed for standard insert if one wants to overwrite default size. Looks good to me now. I will merge this and do some tests later on for it. Then blog post + release. Version here vs version on the PowerShellGallery is a bit outdated. This one supports reading events from Files, Folders, DC's and Forwarders at same time. The old one doesn't do that.

Great PR btw!

[snd3r]

Got it =) I checked it before shipping, overwrite default size with embedding works as it should. Thanks, nice to work with you!