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

NEGRAH GCP fixes #117 #127

Closed
wants to merge 10 commits into from
Closed

NEGRAH GCP fixes #117 #127

wants to merge 10 commits into from

Conversation

NEGRAH-S
Copy link

@NEGRAH-S NEGRAH-S commented Mar 31, 2023

Description

I added few comments from my end that will elaborate more to its understanding as well as I made some changes to indentation.

Changes Made

-Added Comments
-Indentation was done

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

issue

Additional Notes

This was done with lots of hardwork and diligences any update or ammendment will be highly respected.

@google-cla
Copy link

google-cla bot commented Mar 31, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@NEGRAH-S NEGRAH-S changed the title fixes #issuenumber117 NEGRAH GCP fixes #issuenumber117 Mar 31, 2023
@NEGRAH-S NEGRAH-S changed the title NEGRAH GCP fixes #issuenumber117 NEGRAH GCP fixes #117 Mar 31, 2023
@mshudrak
Copy link
Collaborator

Please accept Google CLA before I can review this commit. Adjust commit message to better summarize what you want to change in this PR.

@NEGRAH-S NEGRAH-S changed the title NEGRAH GCP fixes #117 NEGRAH GCP fixes #117 Apr 1, 2023
@Bhardwaj-Himanshu
Copy link
Contributor

Hi @NEGRAH-S, if you are still unable to figure out why Google-CLA is stopping you from making a change.

Ensure both of your Github-account- @NEGRAH-S and your Local-Git-Machine account-<ne***h​@Negrahs-MacBook-Pro.local> are linked with a single Email account.

Also all the commits should be made after first switching your github email to public and then use git config --global user.email "[email protected]" to configure your local repoistory's Email Address.

I hope this helps, cause I just figured it out recently too!
Thanks

Copy link
Collaborator

@ZetaTwo ZetaTwo left a comment

Choose a reason for hiding this comment

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

It's very difficult to follow what changes are where and why. Please squash the commits and separate the whitespace changes from the comment changes so that I can continue the review.

@@ -1,18 +1,19 @@
#!/usr/bin/env python3
'''!/usr/bin/env python3
Copy link
Collaborator

Choose a reason for hiding this comment

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

This whole block is incorrect. Please revert

src/gcp_scanner/__init__.py Show resolved Hide resolved
@@ -15,8 +15,8 @@
"""The main module of GCP Scanner.

"""

# imprted main module of GCP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant comment

from . import scanner

if __name__ == '__main__':
scanner.main()
if __name__ == "__main__":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why " instead of '?


import argparse
import logging

# Defined a function using imported 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 comment

@NEGRAH-S
Copy link
Author

NEGRAH-S commented Apr 7, 2023

Please accept Google CLA before I can review this commit. Adjust commit message to better summarize what you want to change in this PR.

mam i have accepten it but don't know why it is showing like that let me check the linked email for both

@NEGRAH-S
Copy link
Author

NEGRAH-S commented Apr 7, 2023

@ZetaTwo I am working upon the work told by you and will revert u as soon as possible

@Bhardwaj-Himanshu
Copy link
Contributor

Hi @NEGRAH-S, Hope you are doing well mate!
If you are still facing the issue of not being able to fix the "Verification by Google-CLA" issue, I would be happy to help you over that.

@NEGRAH-S
Copy link
Author

@Bhardwaj-Himanshu hey bro, yeah I would highly admire if you could help in same.

@Bhardwaj-Himanshu
Copy link
Contributor

Bhardwaj-Himanshu commented Apr 11, 2023

@NEGRAH-S, I am very happy to help brother!
But first I would require some help from your side.
Could you please share your github email id: the one you use to sign in.
and the share me the information which appears in terminal after typing git config --list

Thanks

@NEGRAH-S
Copy link
Author

@Bhardwaj-Himanshu
My GitHub email- [email protected]
other credential as per my terminal :
Screenshot 2023-04-11 at 7 00 46 PM

@Bhardwaj-Himanshu
Copy link
Contributor

Hi @NEGRAH-S,
Thanks for the images,
Go to github settings now-->emails-->"set my email address to private"(below primary email address options)--> set this either to public.

OR

You can change your "GIT-CONFIG USEREMAIL "

This link is a detailed guide, please do check it out-https://help.umbler.com/hc/en-us/articles/360007180671-Turning-your-e-mail-public-on-GitHub

@NEGRAH-S
Copy link
Author

@Bhardwaj-Himanshu I did as u said then also updated my CLA details it shows updated but still issue is not resolved. can we connect on some private medium for solve the issue.
Screenshot 2023-04-12 at 7 12 40 PM

@Bhardwaj-Himanshu
Copy link
Contributor

Bhardwaj-Himanshu commented Apr 12, 2023

@NEGRAH-S, Thanks for the feedback.

Now that you have updated your email both on your local machine and github--> I want you to check if your email is marked private or public on github, using the link- https://help.umbler.com/hc/en-us/articles/360007180671-Turning-your-e-mail-public-on-GitHub

Coming on to connecting privately, I would love to do so, so here is my linkedin-https://www.linkedin.com/in/himanshu--bhardwaj/

But let's keep this discussion open and public here on github, so that if I am unable to help, others can look into it and help you.

After opting for public email id on github, I want you to again commit the changes first by making a small change on your local machine, and then upload them into github into another branch or trying using same branch even #117 for your forked repository.

Then create another pull request(could be a test one) and let the checks take place and let me know via a screenshot what happens!

@NEGRAH-S
Copy link
Author

As discussed with @Bhardwaj-Himanshu I have decided to close this PR and make new with changes mentioned by @ZetaTwo .

@NEGRAH-S NEGRAH-S closed this Apr 17, 2023
@Bhardwaj-Himanshu
Copy link
Contributor

@mshudrak, He'll open this pull request again, please don't close this until now.
@NEGRAH-S, do link this issue, while create a new PR.

@NEGRAH-S NEGRAH-S reopened this Apr 18, 2023
@NEGRAH-S
Copy link
Author

@mshudrak, He'll open this pull request again, please don't close this until now.
@NEGRAH-S, do link this issue, while create a new PR.

okh bro I got that will link the issue to my new PR this time.

@Bhardwaj-Himanshu
Copy link
Contributor

Also, before changing the lines could you please add "editor.formatOnSaveMode": "modifications",--this line of code to your settings.json file?
What would it do?
-prevent unnecessary introduction of white spaces and blank lines and would only show actual lines modified while doing a commit.
-To do this do Ctrl+Shift+P in vscode --> settings.json in the commmand palette which appears--> open it--> add "editor.formatOnSaveMode": "modifications"--> save

OR you could watch this youtube video for the same, https://www.youtube.com/watch?v=-7t0ewz7-jk

dest="log_file",
help="Save logs to the path specified rather than displaying in\
console",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also try removing the unnecessary whitespaces and indentations, these create a great amount of confusion when reviewing the file and some changes could go unnoticed.

Thanks!

@Bhardwaj-Himanshu
Copy link
Contributor

@NEGRAH-S Do update this issue with your progress.

Or else @mshudrak @ZetaTwo please close this issue as it's unattended!

@NEGRAH-S
Copy link
Author

NEGRAH-S commented May 8, 2023

@NEGRAH-S Do update this issue with your progress.

Or else @mshudrak @ZetaTwo please close this issue as it's unattended!

hello @Bhardwaj-Himanshu , I was occupied with some work but I saw someone has made an PR addressing same issue , so I feel this PR can be closed .

@NEGRAH-S NEGRAH-S closed this May 8, 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.

4 participants