Skip to content

Refactor the parser to handle bulk parsing#49

Draft
ekohl wants to merge 1 commit into
theforeman:masterfrom
ekohl:bulk-strings
Draft

Refactor the parser to handle bulk parsing#49
ekohl wants to merge 1 commit into
theforeman:masterfrom
ekohl:bulk-strings

Conversation

@ekohl

@ekohl ekohl commented Dec 24, 2021

Copy link
Copy Markdown
Member

puppet-strings can parse multiple files at once. This saves a lot of overhead of initializing Puppet over and over.

It does refactor the instance API while keeping the class API the same. There are no tests yet for the new by_name API. A follow up PR to Kafo itself will be submitted.

It also opens the path to using puppet-strings to parse data type aliases and drop that code from Kafo itself. This will fix some nasty bugs with newlines and make things faster by loading everything from the cache.

@ekohl

ekohl commented Dec 24, 2021

Copy link
Copy Markdown
Member Author

This will likely be a major version bump. Perhaps a good chance to rebase on #39.

@ehelms

ehelms commented Jan 3, 2022

Copy link
Copy Markdown
Member

This looks good to me, once you feel it's ready to drop out of draft mode.

If this forces a major version bump, I would also have us consider dropping some older Ruby and Puppet versions (e.g. https://github.com/theforeman/jenkins-jobs/blob/master/theforeman.org/pipelines/test/kafo.groovy#L14) from support.

@ekohl

ekohl commented Jan 3, 2022

Copy link
Copy Markdown
Member Author

A follow up PR to Kafo itself will be submitted.

theforeman/kafo#343

If this forces a major version bump

Technically it doesn't force it: the API used by Kafo remains the same. However, the actual class changes a lot. I think it's a good excuse to do a major version bump and get the things you mention out of the way.

Perhaps we should also reconsider the available? call: we often just run a single parser and only have a single implementation anyway. Perhaps it should just raise an exception if it's unavailable. However, that would be more breaking I think.

puppet-strings can parse multiple files at once. This saves a lot of
overhead of initializing Puppet over and over.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants