Skip to content

virts-2979 - Improve Debrief's fact reporting #54

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

Merged
merged 1 commit into from
Nov 2, 2021

Conversation

ArtificialErmine
Copy link
Contributor

Description

Currently, the debrief plugin has a minor issue in that it scans links in order to generate the collection of facts for an operation, rather than leverage "all_facts". This can lead to minor discrepancies, which this patch addresses by updating debrief to use the new approach to fact retrieval. There are also various minor flake fixes that got caught up in this.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

This change has been tested manually, with the Snag Broadcast IP ability, and with the complete test battery.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • [NA] I have made corresponding changes to the documentation
  • [NA] I have added tests that prove my fix is effective or that my feature works

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@clenk clenk left a comment

Choose a reason for hiding this comment

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

Looks good to me, assuming the corresponding PR on caldera core goes in. PDF didn't have any learned facts when I tested it without that PR.

Copy link

@yee-jonathan yee-jonathan left a comment

Choose a reason for hiding this comment

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

Reviewed and don't see any perceived issues or conflicts. Added the changes to local instance and didn't run into any errors.

@ArtificialErmine ArtificialErmine merged commit b566c0a into master Nov 2, 2021
@ArtificialErmine ArtificialErmine deleted the virts-2979 branch November 2, 2021 13:07
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