(SDK-137) Add puppet syntax validation #105
Conversation
bmjen
force-pushed
the
validation-cleanup
branch
from
f5407b7
to
8fda2db
Compare
lib/pdk/report.rb
Outdated
| @@ -85,6 +85,9 @@ def to_junit(target = self.class.default_target) | |||
| # @param target [#write] an IO object that the report will be written to. | |||
| # Defaults to PDK::Report.default_target. | |||
| def to_text(target = self.class.default_target) | |||
| # Extra defaulting here, b/c the Class.send method will pass in nil | |||
| target ||= self.class.default_target | |||
There was a problem hiding this comment.
This method looks like its primary concern should be formatting, not outputting. You can get rid of this dance here, and make the method more flexible, by just returning the final string, and leave IO to the caller.
Alternatively rename it to write_text_to, to capture its intent. Compare with to_hash returning a Hash, not IO.
There was a problem hiding this comment.
Good idea. And the formatting happens in the to_text method of the Event class.
| end | ||
|
|
||
| def self.sanitize_console_output(line) | ||
| line.gsub!(%r{\e\[([;\d]+)?m}, '') |
There was a problem hiding this comment.
Can this be avoided by setting Puppet[:color] = false in https://github.com/voxpupuli/puppet-syntax/blob/master/lib/puppet-syntax/manifests.rb#L19 , or a similar place ?
There was a problem hiding this comment.
I wouldn't want to change the default behavior of puppet-syntax itself and strip the colors, since users of this tool outside of PDK may still want the distinct console colors. I could add a Rakefile option to disable the colors in puppet-syntax and toggle it on or off based on that.
DavidS
changed the title
|
Still needs a rebase :-/ |
|
👍 |
bmjen
force-pushed
the
validation-cleanup
branch
from
1572771
to
49fdbb6
Compare
Also includes some refactoring of the validation and output parsing.
rodjek
force-pushed
the
validation-cleanup
branch
4 times, most recently
from
5a7c1c2
to
96bcd77
Compare
rodjek
force-pushed
the
validation-cleanup
branch
from
96bcd77
to
15e00be
Compare
rodjek
force-pushed
the
validation-cleanup
branch
from
e4b451c
to
9c744c3
Compare
|
Made a couple of changes to fix up the acceptance tests on Windows:
|
|
An alternative to disabling warnings would be to remove matching known problematic deprecation warnings and removing them from the output before parsing, but that seems like a brittle solution. Group consensus on the least awful solution? :) |
|
Filtering the known warnings for test purposes would be preferable, as it would allow us to detect new warnings on incoming code before it hits master. No reason to block this PR for it though. |
This feature requires this fix voxpupuli/puppet-syntax#83 to be published.