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.

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

Changing opa cli flag descriptions to match online docs #2587

Merged
merged 3 commits into from
Aug 6, 2020
Merged

Changing opa cli flag descriptions to match online docs #2587

merged 3 commits into from
Aug 6, 2020

Conversation

OmegaVVeapon
Copy link
Contributor

@OmegaVVeapon OmegaVVeapon commented Jul 30, 2020

  • Changed the data flag description to specify that it can take policy
    or data files
  • Suffixed some descriptions with "This flag can be repeated." where
    applicable

Fixes: #2586
Signed-off-by: Alan Silva alansq16@gmail.com

@ashutosh-narkar
Copy link
Member

Thanks @OmegaVVeapon for submitting these changes. We should update the opa deps commands to use the addDataFlag and addBundleFlag methods similar to the opa eval command. We could also add a long description to the deps command similar to eval to include usage examples.

@tsandall
Copy link
Member

tsandall commented Aug 4, 2020

@OmegaVVeapon can you update the deps command to use the addDataFlag and addBundleFlag functions? Once that change is done, we can merge this.

@OmegaVVeapon
Copy link
Contributor Author

Ah, my apologies, I misunderstood the comment w/r/t those functions as a refactor change that would happen separately from this PR.

I'm no Go developer but I'll give it a try @tsandall .

- Changed the data flag description to specify that it can take policy
or data files
- Suffixed some descriptions with "This flag can be repeated." where
applicable

Fixes: #2586
Signed-off-by: Alan Silva <alansq16@gmail.com>
Signed-off-by: Alan Silva <alansq16@gmail.com>
Signed-off-by: Alan Silva <alansq16@gmail.com>
@OmegaVVeapon
Copy link
Contributor Author

OmegaVVeapon commented Aug 6, 2020

Done. Let me know if the commits are alright.

I also added one more function, the addOutputFormat function, in the spirit of further unifying the opa eval and opa deps code.

If that's undesired, I'll revert the commit.

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this!

@tsandall tsandall merged commit b659f15 into open-policy-agent:master Aug 6, 2020
@OmegaVVeapon OmegaVVeapon deleted the matching-flag-descriptions-with-online-docs branch August 6, 2020 19:43
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.

Some of the opa cli flag descriptions don't match the online documentation
3 participants