oracle / yo

A fast and simple command line OCI client
Universal Permissive License v1.0
63 stars 6 forks source link

Adding ssh-copy-id wrapper #18

Closed AnuravModak closed 1 year ago

AnuravModak commented 1 year ago

Towards #16 So following up on our last discussion about the above referenced issue, I have made changes in accordance to your suggestions. Please review the code and provide feedback.

AnuravModak commented 1 year ago

I have also made the changes to add my signature for the changes like this

image
brenns10 commented 1 year ago

Ok, several things here.

You've added commits to your branch, but that doesn't change the fact that the old commits, which still contain problems, are present on the branch. Rather than add commits that correct previous errors, the better approach (which this project uses) is to amend your commits to remove the error from the original commits. Your branch needs to be in a state such that you can run git log see just a single commit that looks something like this:

commit 72a265f053e4b7dc9815d36584d5153d00000d8b
Author: Anurav Modak <your.email@oracle.com>
Date:   Wed Sep 6 10:06:41 2023 -0700

    Implement ssh-copy-id command    

    Signed-off-by: Anurav Modak <your.email@oracle.com>

There should be no fixup commits or anything else, just your one commit, and then the commits that you branched from.

Second, you've committed using the Github web editor. This sets the "author" field in git to Anurav Modak <79191029+AnuravModak@users.noreply.github.com> which is going to upset the Oracle Contributor Agreement bot, more on that in a moment. Please make all commits from your computer, not the Github editor.

Third, you've used a gmail address to commit all this code. Policy requires that you contribute via your Oracle email address, since you're an employee. You should switch your Git configuration to use your correct Oracle email address -- you can do it for just this repository with git config --local user.email your.email@oracle.com. Or you can set it globally with --global.


To get things in a good state, I'd recommend the following:

# Make sure the upstream repository is present as a remote
$ git remote add upstream git@github.com:oracle/yo
$ git fetch upstream

# Use make development to ensure pre-commit hooks are installed
$ make development

# Make sure you've updated your git email address:
$ git config --local user.email your.email@oracle.com

# Now, rebase your branch interactively so you can squash the commits:
$ git rebase -i upstream/main

# In the text editor which appears, keep the first line unmodified.
# For each subsequent line which looks like `"pick [commit id] [commit message"]`,
# replace the word "pick" with `fixup`, then save and exit.

# At this point, you have a branch with a single commit based on the latest main.
# But we still need to correct the code style changes. We can back-out our commit
# and re-do it now that the pre-commit hooks are installed.
$ git reset --soft HEAD^

# All your changes are now staged. We can try to commit. Be sure to use "-s" to sign
# off.
$ git commit -s

# Pre-commit hooks will now apply code style rules and static checks. Here's what I got:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted yo/main.py

All done! ✨ 🍰 ✨1 file reformatted.

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

yo/main.py:1565:9: F841 local variable 'e' is assigned to but never used

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

yo/main.py:1549: error: "defaultdict[str, str]" not callable  [operator]
Found 1 error in 1 file (checked 1 source file)

Reorder python imports...................................................Passed
Check minimum Python version.............................................Passed

# Stage the code style changes:
$ git add yo/main.py

# You'll need to fix the flake8 error and the mypy error as well.
# They should be straightforward but feel free to check with me if you
# have trouble.

# Commit the fixed code
$ git add yo/main.py
$ git commit -s
# Be sure to use a descriptive commit message, like "Add ssh-copy-id command"

# In order to push your branch, you'll need to use "-f" which indicates that you've
# edited the branch and want to force-push the edited history
$ git push -f origin HEAD

If, in the future, you're asked to make fixes to your code during the code review process, the best way would be to amend the commit, rather than adding new ones:

git commit --amend --sign-off
brenns10 commented 1 year ago

Closing this in favor of #20