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

Improved Indentation and added Comments in the codebase. #133

Closed
wants to merge 25 commits into from

Conversation

ro4i7
Copy link
Contributor

@ro4i7 ro4i7 commented Apr 6, 2023

Description

Fixes #117

Improved Indentation and added Comments in the codebase.

Please give wherever changes need.

Changes Made

[List the changes you have made in bullet points.]

Checklist

  • I have read and followed the contributing guidelines.
  • I have tested my changes thoroughly and they work as expected.
  • I have added necessary tests for the changes made.
  • I have updated the documentation to reflect the changes made.
  • My code follows the project's coding style and standards.
  • I have added appropriate commit messages and comments for my changes.

Related Issues

[If your changes relate to any existing issues, please list them here.]

Additional Notes

[Provide any additional notes or context that may be helpful for the reviewers.]

Feel free to modify and customize this template as needed to fit the specific needs of the GCP Scanner project.

@ro4i7
Copy link
Contributor Author

ro4i7 commented Apr 6, 2023

@mshudrak please check this

@mshudrak
Copy link
Collaborator

mshudrak commented Apr 6, 2023

Well, the pylint test is failing now. Could you remind me why do you want 4 spaces?

@ro4i7
Copy link
Contributor Author

ro4i7 commented Apr 7, 2023

@mshudrak ,in the review section, there is no showing pylint error for other files except credsdb.py is there any check still needed?

@mshudrak
Copy link
Collaborator

mshudrak commented Apr 7, 2023

I don't see that tests even run now. Additionally, this CL is too large for review. I'd advice you to split it per file. I also see a lot of comments you added to be redundant. See one I made for example.

@@ -14,5 +14,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Import the scanner module from the gcp_scanner package
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is redundant


# Call the main function of the scanner module to start the scanning process
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is redundant

@@ -16,7 +16,10 @@

"""

# Importing the scanner module
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant

if __name__ == '__main__':
# Calling the main function of the scanner module
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is redundant

@@ -20,6 +20,8 @@
import argparse
import logging


# Define a function to create an argument parser using the argparse module
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is redundant

@@ -31,102 +33,117 @@ def arg_parser():
argparse.Namespace: A namespace object containing the parsed command-line
arguments.
"""
# Create a new parser object
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is redundant. I am stopping on this one. You do not need to comment obvious sections of the code.

@ro4i7
Copy link
Contributor Author

ro4i7 commented Apr 7, 2023

I don't see that tests even run now. Additionally, this CL is too large for review. I'd advice you to split it per file. I also see a lot of comments you added to be redundant. See one I made for example.

Yeah. I also thought that. Ok, I am working on it

@ro4i7
Copy link
Contributor Author

ro4i7 commented Apr 14, 2023

I don't see that tests even run now. Additionally, this CL is too large for review. I'd advice you to split it per file. I also see a lot of comments you added to be redundant. See one I made for example.

Yeah. I also thought that. Ok, I am working on it

I think, can I close this issue. And start new PR fir group of 3 files. So that it can easy for check?

@mshudrak
Copy link
Collaborator

Take a look at my other comments, a lot of comments are redundant. We do not need that level of comment details especially for obvious things. Also, we use 2 spaces instead of 4.

@mshudrak
Copy link
Collaborator

mshudrak commented Jun 5, 2023

Closing this one due to inactivity. Feel free to split it into small PRs and resubmit.

@mshudrak mshudrak closed this Jun 5, 2023
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.

Improving Indentation and adding Comments in codebase
2 participants