Skip to content

Maintenance refactoring#7

Merged
akofink merged 1 commit intoOpenSCAP:masterfrom
xprazak2:refactor-parse
Aug 7, 2019
Merged

Maintenance refactoring#7
akofink merged 1 commit intoOpenSCAP:masterfrom
xprazak2:refactor-parse

Conversation

@xprazak2
Copy link
Copy Markdown
Collaborator

@xprazak2 xprazak2 commented Aug 6, 2019

Extracts rule results and test result
into a separate concenrns.

Improves how newlines are removed from text.

Copy link
Copy Markdown
Collaborator

@akofink akofink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 👍 Just a couple comments.

Comment thread lib/openscap_parser/test_result.rb
Comment thread lib/openscap_parser/rule.rb Outdated
Extracts rule results and test result
into a separate concerns.

Improves how newlines are removed from text.
@xprazak2
Copy link
Copy Markdown
Collaborator Author

xprazak2 commented Aug 7, 2019

Updated with suggested changes.

Copy link
Copy Markdown
Collaborator

@akofink akofink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK Thanks @xprazak2! (I have one other question inline)

def identifier
@identifier ||= {
label: @rule_xml.at_css('ident')&.text,
label: @rule_xml.at_css('ident') && @rule_xml.at_css('ident').text,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? You don't like the new &. syntax?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the proxy has to run on an older version of ruby...?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - I'm not blocking this PR on it, just curious.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, proxies may have older versions of ruby.

@akofink
Copy link
Copy Markdown
Collaborator

akofink commented Aug 7, 2019

Closing and reopening to trigger tests

@akofink akofink closed this Aug 7, 2019
@akofink akofink reopened this Aug 7, 2019
@akofink akofink merged commit d0340dd into OpenSCAP:master Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants