Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Powershell 2.0 compatibility #20

Open
wants to merge 3 commits into
base: PS2.0-Compat
Choose a base branch
from

Conversation

rattuscz
Copy link
Contributor

Initial try to bring script to PS 2.0 compatible version - as our ADFS 2.0 are also PS2.0 😢

Analytics part still does not work - ConvertFrom-Json not in ps2

Not sure if this PS2.0 compatibility is even desirable in main branch, maybe as a separate?
Anyway looking for feedback

Analytics part still does not work ConvertFrom-Json not in ps2
@msftclas
Copy link

msftclas commented May 28, 2018

CLA assistant check
All CLA requirements met.

@bongiovimatthew-microsoft
Copy link
Contributor

@rattuscz, this looks like a fantastic start on the PS 2.0 compat. To handle the lack of ConvertFrom-Json, you might have to construct PowerShell objects to correspond to the template that we use.

I also think you have a good point about the PS 2.0 compat code not making sense in the master branch. One downside to having a dedicated sub-branch is that it will have to be periodically updated with the latest changes going into master. It might make sense for you to maintain a forked project, which we can call out in the documentation on our project. That would mean you would be the owner, and could manage the merging of any updates.

@rattuscz
Copy link
Contributor Author

@bongiovimatthew-microsoft I'm right in the middle of switching places inside company so in one-two months I probably won't even have access to any servers with ADFS to test it on. That would probably make me a very poor owner 🙁

I will be happy to finish with replacing the json templates and any other problem I find to finish this PR as working PS2 version.

Also the line foreach ( $Event in $Result | Where { $null -ne $_ } ) - I'm not sure this is PS2 only problem, as $Result is @() + Get-WinEvent call that can lead to @() + $null which is array with null inside -> causes errors inside the foreach loop when not checked.

@bongiovimatthew-microsoft
Copy link
Contributor

Thanks @rattuscz, I created a new branch for the PS 2.0 compat called PS2.0-Compat. If you wouldn't mind switching this PR over to that branch, that would be great.

Also, for the loop over $Result, you're right that it could be an array with $null inside, and that doesn't seem to be just a PS2 problem. Would you mind adding that one-line fix into your other open PR?

@rattuscz rattuscz changed the base branch from master to PS2.0-Compat June 1, 2018 15:40
@rattuscz
Copy link
Contributor Author

rattuscz commented Jun 1, 2018

I've had some other pressing issues so I will get to it next week.

I will create separate PR for the $null filter.

Also have to chase someone to get the CLA approval :/

@rattuscz
Copy link
Contributor Author

rattuscz commented Jun 5, 2018

ok separate PR for the $null done, branch switched to PS2.0-Compat

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants