Skip to content
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

Do not crash when referring to a fact key without quoting it #119

Merged
merged 3 commits into from
Jun 9, 2023

Conversation

chutzimir
Copy link

Fixes #118

@chutzimir chutzimir requested a review from a team as a code owner April 19, 2023 01:12
@chelnak
Copy link

chelnak commented Apr 19, 2023

@chutzimir This is awesome. Thank you for taking the time to submit this patch.

Please could you add a spec test to demonstrate the case & then I will merge.

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2023

CLA assistant check
All committers have signed the CLA.

@chutzimir
Copy link
Author

@chelnak I missed your message, but added the spec just now, also rebased on main.

@chelnak
Copy link

chelnak commented Apr 25, 2023

@chutzimir Thanks! Looks like you have some rubocop errors to fix before we can merge.

Please could you take a look?

@chutzimir
Copy link
Author

chutzimir commented Apr 25, 2023

@chutzimir Thanks! Looks like you have some rubocop errors to fix before we can merge.

Please could you take a look?

@chelnak I think I addressed all of them. Should I squash those commits or is it OK as it is?

@chelnak
Copy link

chelnak commented Apr 25, 2023

Yes- could be great if you could clean up those commits a bit.

Thanks!

@chutzimir
Copy link
Author

Yes- could be great if you could clean up those commits a bit.

Thanks!

Done. It is three commits now, with each of them meaningful and independent, bringing its own value.

@chelnak
Copy link

chelnak commented Apr 26, 2023

Oh no the rubocop errors have reappeared!

@chutzimir
Copy link
Author

Oh no the rubocop errors have reappeared!

Wack!

I now learned how to run rubocop locally before pushing. It was clean for me, let's see if the official pipeline agrees.

@chelnak chelnak merged commit 9729bb5 into puppetlabs:main Jun 9, 2023
@chelnak chelnak added the bug Something isn't working label Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when trying to parse facts with unquoted keys
4 participants