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

update(docs): improve information in the configuration docs #5829

Merged
merged 9 commits into from
Sep 28, 2022

Conversation

VladMasarik
Copy link
Contributor

Proposed Changes

  • Docs suggestions. When I was trying to setup docs, I had to use the online docs, and finding out anything was an awful pain especially because I had to search through the code.

I did not create an issue first because having something concrete to comment about is faster than explaining my suggestions, and then implementing them. The PR is of course not final, I would like to first know you thoughts, improvements, and I have a few questions. When all is done, I would adjust the JSON, TOML etc. parts as well.

Questions:

  • Why is the section "Using commands on scanned files as comments" in "Running KICS" rather than configuration section?
  • Does verbse flag work? When we set it verbose=false there was no difference with verbose=true that we could see.
  • type: "type of queries to use in the scan" does that require list of queries?

I submit this contribution under the Apache-2.0 license.

Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
@rafaela-soares rafaela-soares added documentation Improvements or additions to documentation community Community contribution labels Sep 23, 2022
@VladMasarik
Copy link
Contributor Author

Also, I am happy to do more changes in the docs in this PR if consistency with other docs would be an issue, just let me know.

Copy link
Contributor

@rafaela-soares rafaela-soares left a comment

Choose a reason for hiding this comment

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

Hello, @VladMasarik!

Thank you so much for using KICS and also contributing 🚀

You can find a more detailed documentation about the scan flags here.

@rafaela-soares rafaela-soares changed the title Improve information in the configuration docs update(docs): improve information in the configuration docs Sep 23, 2022
@VladMasarik
Copy link
Contributor Author

You can find a more detailed documentation about the scan flags here.

Oh so that is where all the explanations are! Completely my opinion, but wouldn't it be better to have this information in the configuration part as well? Or at least I think that a link in the configuration saying ... "Full configuration options can be found in CLI, and if you want to disable kics in some parts of files see Running KICS", would immensely help. I would say duplicating the information would be good as well, but I don't think it is worth the extra maintenance from your side.

Also, another topic would be could KICS have only docs for actually "getting started" in the getting started section? Rather than basically all the docs? Eg. I would expect maybe installation, running KICS examples and references to the actual configuration, and how to read results than all this:
image

Then again I think this would be too much conversation in a single PR.

@rafaela-soares
Copy link
Contributor

You can find a more detailed documentation about the scan flags here.

Oh so that is where all the explanations are! Completely my opinion, but wouldn't it be better to have this information in the configuration part as well? Or at least I think that a link in the configuration saying ... "Full configuration options can be found in CLI, and if you want to disable kics in some parts of files see Running KICS", would immensely help. I would say duplicating the information would be good as well, but I don't think it is worth the extra maintenance from your side.

We totally agree with you! The configuration documentation should be definitely improved. We can present an example of a configuration and refer the command's doc as you suggested. Do you mind updating?

I would like to thank you for noticing, taking the time to contribute, and give your feedback.

Also, another topic would be could KICS have only docs for actually "getting started" in the getting started section? Rather than basically all the docs? Eg. I would expect maybe installation, running KICS examples and references to the actual configuration, and how to read results than all this: image

We will discuss this matter with the Product Manager. As soon as we have some decision, I will let you know. Once again, thank you so much for your feedback 🚀

Then again I think this would be too much conversation in a single PR.

You can use the GitHub discussions or contact us via kics@checkmarx.com, if you want 😊

Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
Signed-off-by: Vladimir Masarik <vladimir.masarik@ef.com>
@VladMasarik VladMasarik marked this pull request as ready for review September 26, 2022 15:35
@VladMasarik
Copy link
Contributor Author

I have added all the changes that I have "imagined". Let me know what you think, and what would need to be adjusted. 👍

Copy link
Contributor

@rafaela-soares rafaela-soares left a comment

Choose a reason for hiding this comment

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

Thank you so much, @VladMasarik! It seems great!

Regarding the "Getting Started" section on the website, we will try to take care of it as soon as possible. We agree that should be improved. Thank you so much for the suggestion!

@rafaela-soares
Copy link
Contributor

Does verbose flag work? When we set it verbose=false there was no difference with verbose=true that we could see.

@VladMasarik, yes. The verbose flag gives information about the logs (WRN, INF, ...). If you need help with that, let me know. For example:

With -v:

image

@rafaela-soares rafaela-soares merged commit 9fdfd61 into Checkmarx:master Sep 28, 2022
@VladMasarik
Copy link
Contributor Author

@rafaela-soares

Regarding the "Getting Started" section on the website, we will try to take care of it as soon as possible.

I dont think you need to rush it, but it was somewhat confusing to see all of the documentation in the "Getting Started" section.

The verbose flag gives information about the logs (WRN, INF, ...). If you need help with that, let me know. For example:

Maybe I was making a mistake somewhere, but I could swear that I still saw these WARN messages. Although, we did enable only WARN and above warning messages.
image

Still thank you for such a quick responses!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants