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

Simplify controls.json format #63

Merged
merged 1 commit into from
Sep 1, 2020
Merged

Simplify controls.json format #63

merged 1 commit into from
Sep 1, 2020

Conversation

alfinkel
Copy link
Contributor

@alfinkel alfinkel commented Aug 31, 2020 •
edited
Loading

What

Simplify expected controls.json format.

Why

The original format isn't intuitive and is overly complex.

How

  • Simplified/flattened the expected content to take a check's class path as a key and a list of accreditations as the value.
  • Ensured original format is still valid.
  • Added supporting docs.
  • Added ControlDescriptor unit tests (squashed two bugs that have been there for 4 years ;) - yeay unit tests!

Test

  • Unit tests succeed as expected
  • Code executed in a venv where old format, new format and a combo were used and all scenarios produced the same expected check results.

Context

@alfinkel alfinkel force-pushed the 51-rationalize-controls-content branch 7 times, most recently from 0630e98 to 08c4ca9 Compare August 31, 2020 19:46
drsm79
drsm79 previously approved these changes Sep 1, 2020
View reviewed changes
Copy link
Contributor

@drsm79 drsm79 left a comment

Choose a reason for hiding this comment

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

Couple of minor nits

doc-source/design-principles.rst Outdated Show resolved Hide resolved
doc-source/design-principles.rst Outdated Show resolved Hide resolved
@alfinkel alfinkel force-pushed the 51-rationalize-controls-content branch from 08c4ca9 to 8b0956c Compare September 1, 2020 14:31
@alfinkel alfinkel merged commit b96470b into main Sep 1, 2020
@alfinkel alfinkel deleted the 51-rationalize-controls-content branch September 1, 2020 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drsm79 drsm79 drsm79 approved these changes

@cletomartin cletomartin Awaiting requested review from cletomartin cletomartin is a code owner

@mattwhite mattwhite Awaiting requested review from mattwhite mattwhite is a code owner

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

Successfully merging this pull request may close these issues.

Rationalize controls content
2 participants
@alfinkel @drsm79