Conversation
183ac28 to
d589ff1
Compare
jalseth
approved these changes
Sep 9, 2020
Collaborator
jalseth
left a comment
There was a problem hiding this comment.
Looks great to me! Sorry for the delay in getting to the review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This spawned from reading through our current docs and shoring up anything that was out of date. Working through a couple of inconsistencies, it just felt better to try to continue to make more sense out of our packages.
The goal was more or less to have a no-op when it comes to changes in outputs (i.e. generated policies, docs) and focus more on the refactoring effort to make review easier.
The only notable set of changes comes from adding the hyphen to each policy which results from removing the newline when printing the source (see chomping). This makes the output less opinionated.
This should provide a more consistent experience when parsing data from each of the policies. Each command can then take the extracted data, and manipulate/use it in a way that makes sense for the command.
Freebies
Library folder no longer matters. The rego files are loaded and the dependencies are built out based on the name of the package(s) that the rego file imports.
The loaded rego files are now be compiled during the
createstep. This makes sure that the entire set of rego policies are actually valid before runningcreate. Previously, no validation was performed.Since all rego is pre-loaded, this allows for package consumers to use the methods on the Rego type without error checking. e.g.
rego.Libraries()Overall this should give more cohesion to the rego package and keep the CLI concerns separate from the package. e.g. The create command no longer needs to figure out the library directory, or call GetKinds/GetName--It's all a part of the loaded rego type.
NOTE
Since the docs/policies/etc are left mostly unchanged and this is more of a refactoring effort--it may be better to view the structure/code on GitHub rather than trying to make sense of the diffs.