poshbotio / PoshBot

Powershell-based bot framework

MIT License

540 stars 108 forks source link

Add initial get-it-working stateful data functions #31

Closed RamblingCookieMonster closed 7 years ago

RamblingCookieMonster commented 7 years ago

Add initial get-it-working stateful data functions

Description

This adds Get-PoshBotStatefulData, Set-PoshBotStatefulData, and Remove-PoshBotStatefulData functions.

Related Issue

30

Motivation and Context

These functions enable plugin authors to maintain state between commands and plugins.

How Has This Been Tested?

With the included test task

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project. (Maybe. I tried.)
[x] My change requires a change to the documentation. (TODO)
[x] I have updated the documentation accordingly. (Comment based help. Later: external docs)
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

RamblingCookieMonster commented 7 years ago

Will untag as a work-in-progress when I've wrapped up the TODOs and rebased.

This was more of a 'get-it-working' bit. Happy to make changes as needed (e.g. filenames, anything else)

Cheers!

RamblingCookieMonster commented 7 years ago

Alrighty! This is at a point where I'd be comfortable with someone looking it over. Uncovered a bug or two running through the tests, as usual : )

Also, amended to original note to include Remove-PoshBotStatefulData - otherwise, once we write stateful data, it will always exist - even if we set a property to $null, it would still retain the property node.

Cheers!

devblackops commented 7 years ago

Nice work @RamblingCookieMonster!

It looks like you're referencing $pbc to get the ConfigurationDirectory value but that object isn't initialized anywhere. I assume that was created from following the basic usage example in the README?

I think the $global:PoshBotContext object should be modified a bit to include ConfigurationDirectory (maybe more?) so that becomes the only external data that these new functions need.

Thoughts?

RamblingCookieMonster commented 7 years ago

Indeed! I was running an exploratory PoshBot command (get-variable output and so forth) and found that. Turns out it's the variable I use to load up the config ahead of time, eek!

Yeah, would be handy to have that accessible via PoshBotContext! Want to tackle that in a separate branch? In here? Can I help? : )

devblackops commented 7 years ago

I just pushed commit 72717e12ad283be7bc4e89b1eb43c66f1abf5f56 and 0d77f3ef918908b257fda94dde9907439fd746bb with these changes. Basically, ConfigurationDirectory is now a property of $global:PoshBotContext and the ParsedCommand object is a property instead of being the only value. I duplicated a few of the properties of ParsedCommand at the top level for convenience as well.

If you update the PR to use $global:PoshBotContext.ConfigurationDirectory instead of $pbc I think this is ready to merge!

Cheers.

devblackops commented 7 years ago

Thanks @RamblingCookieMonster!

I made a change to Set-PoshBotStatefulData so it accepts any object and not just [string] but other than that nice work!

RamblingCookieMonster commented 7 years ago

Sweet! Oops, yeah, value should have allowed arbitrary input, thanks for the fix!