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.

Sign up for GitHub

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

Jump to bottom

fix: parameter aggregation fix #1443

Merged
merged 15 commits into from
Sep 12, 2023
Merged

fix: parameter aggregation fix #1443

merged 15 commits into from
Sep 12, 2023

Conversation

AleJo2995
Copy link
Collaborator

Types of changes

  • Hot fix (emergency fix and release)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation (change which affects the documentation site)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Release (develop -> main)

Quality assurance (all should be covered).

  • My code follows the code style of this project.
  • Documentation for my change is up to date?
  • My PR meets testing requirements.
  • All new and existing tests passed.
  • All commits are signed-off.

Summary

Fixes #1442

Key links:

Before you merge

  • Ensure it is a 'squash commit' if not a release.
  • Ensure CI is currently passing
  • Check sonar. If you are working for a fork a maintainer will reach out, if required.

@AleJo2995
fix: parameter aggregation fix
bb14688 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: arranging tests
a2ed13a 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995 AleJo2995 requested a review from mrgadgil September 4, 2023 21:13
@AleJo2995 AleJo2995 marked this pull request as ready for review September 4, 2023 23:24
@AleJo2995
fix: triggering build
bac6675 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: increase time out for cache response
098fed3 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
vikas-agarwal76
vikas-agarwal76 reviewed Sep 5, 2023
View reviewed changes
tests/trestle/core/commands/author/ssp_test.py Show resolved Hide resolved
trestle/common/const.py Show resolved Hide resolved
trestle/core/catalog/catalog_writer.py Show resolved Hide resolved
trestle/core/catalog/catalog_writer.py Show resolved Hide resolved
trestle/core/commands/author/profile.py Outdated Show resolved Hide resolved
trestle/core/commands/author/profile.py Outdated Show resolved Hide resolved
@AleJo2995
Copy link
Collaborator Author

A comment to review - only values, display-name (if modified in markdown) and param-id will appear in final assembled-profile

@AleJo2995
Merge branch 'develop' into fix/paramater-aggregation
d4106a4
@AleJo2995
Merge branch 'develop' into fix/paramater-aggregation
cd53ac6
@AleJo2995
fix: profile-values are shown in markdown even when there are values …
5f1bbd2 
…already

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: adding alt identifier validation
c77ae13 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: correct profile values validation
01ce7ee 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: remove parameter aggregation from assembly and remove label bein…
deca2f5 
…g shown in assembled profile

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995
fix: correcting test failures and various formatting issues
e6134af 
Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
vikas-agarwal76
vikas-agarwal76 reviewed Sep 9, 2023
View reviewed changes
trestle/core/catalog/catalog_reader.py
param_dict[const.PROFILE_VALUES].remove('<REPLACE_ME>')
param_dict[const.VALUES] = param_dict[const.PROFILE_VALUES]
if not write_mode:
param_dict.pop(const.PROFILE_VALUES)
final_param_dict[param_id] = param_dict
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the additional check for write mode? it was not there earlier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is meant to not write replace me or pop profile values in read mode. Meaning that while writing into markdown replace me is not going to be removed or profile values are not as well. Logic before was implemented differently to pop the value out

trestle/core/catalog/catalog_reader.py Outdated Show resolved Hide resolved
trestle/core/catalog/catalog_writer.py Show resolved Hide resolved
vikas-agarwal76
vikas-agarwal76 approved these changes Sep 12, 2023
View reviewed changes
Copy link
Collaborator

@vikas-agarwal76 vikas-agarwal76 left a comment

Choose a reason for hiding this comment

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

LGTM

@AleJo2995 AleJo2995 merged commit dd9e3bc into develop Sep 12, 2023
16 checks passed
@AleJo2995 AleJo2995 deleted the fix/paramater-aggregation branch September 12, 2023 13:55
jpower432 pushed a commit to RedHatProductSecurity/compliance-trestle that referenced this pull request Sep 18, 2023
@AleJo2995 @jpower432
fix: parameter aggregation fix (oscal-compass#1443)
a391fcc 
* fix: parameter aggregation fix

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: arranging tests

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: triggering build

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: increase time out for cache response

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: profile-values are shown in markdown even when there are values already

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: adding alt identifier validation

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: correct profile values validation

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: remove parameter aggregation from assembly and remove label being shown in assembled profile

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: correcting test failures and various formatting issues

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: change order for parameters in markdown

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: not setting empty values

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

---------

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
jpower432 pushed a commit to jpower432/compliance-trestle that referenced this pull request Sep 19, 2023
@AleJo2995 @jpower432
fix: parameter aggregation fix (oscal-compass#1443)
56a3409 
* fix: parameter aggregation fix

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: arranging tests

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: triggering build

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: increase time out for cache response

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: profile-values are shown in markdown even when there are values already

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: adding alt identifier validation

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: correct profile values validation

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: remove parameter aggregation from assembly and remove label being shown in assembled profile

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: correcting test failures and various formatting issues

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: change order for parameters in markdown

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

* fix: not setting empty values

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>

---------

Signed-off-by: Alejandro Jose Leiva Palomo <[email protected]>
@AleJo2995 AleJo2995 mentioned this pull request Sep 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikas-agarwal76 vikas-agarwal76 vikas-agarwal76 approved these changes

@mrgadgil mrgadgil Awaiting requested review from mrgadgil

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parameter aggregation failure
2 participants
@AleJo2995 @vikas-agarwal76