search

puppetlabs / ruby-pwsh

A ruby gem for interacting with PowerShell
MIT License
15 stars 23 forks source link

Update dsc_base_provider.rb #204

Closed Hvid closed 1 year ago

Hvid commented 1 year ago

Fixes https://github.com/puppetlabs/ruby-pwsh/issues/203 and https://github.com/puppetlabs/Puppet.Dsc/issues/242

This is just copied from the comment by https://github.com/ShawnHardwick

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

chelnak commented 1 year ago

Hi! Looks like there are some rubocop offences. Can you check them out please?

Hvid commented 1 year ago

I tried fixing the rubocop errors.

Hvid commented 1 year ago

I don't know how to fix the tests.

Hvid commented 1 year ago

As i said in the original comment, this is just a copy+paste from the workaround that was posted. I would like someone from the team to review if this is in fact the correct solution. I tested it and in my case it solves the issue.

chelnak commented 1 year ago

It seems to make sense but I would like to look at this closely before merging.

Just to be sure it doesn't introduce any other issues.

jordanbreen28 commented 1 year ago

Nice one @Hvid! Just taking a quick look at the spec tests, there tests are still referring to the modified instance variables as class variables.

updating this should help rid some errors described_class.class_variable_set(:@@cached_canonicalized_resource, nil) described_class.instance_variable_set(:@cached_canonicalized_resource, [])

Likewise, updating each instance of class_variable_get to instance_variable_get

Hvid commented 1 year ago

Thanks @jordanbreen28 - There is a bunch of new errors now. Can you please have a quick look again?

jordanbreen28 commented 1 year ago

Hey @Hvid apologies for the delay in getting back! Did you get any further with these tests? I've had a quick look, and it seems that the test cases will need some further tweaking to pass.

Happy to collaborate with you if you encounter any issues!

Hvid commented 1 year ago

I'm sorry, I don't have the ncessary ruby skills to fix those errors.

jordanbreen28 commented 1 year ago

I'm sorry, I don't have the ncessary ruby skills to fix those errors.

Not to worry.

Will leave this PR open so it stays on our radar and list of things to-do, and in the meantime might attract some contributions to help get it over the line.

jordanbreen28 commented 1 year ago

Apologies @Hvid, I'm opting to close this in favour of https://github.com/puppetlabs/ruby-pwsh/pull/234 due to the conflicts present on your branch. Thank you for giving us a great point to continue from!