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

Resolving issues #1, #2, and #4 #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

solonovamax
Copy link

@solonovamax solonovamax commented Aug 6, 2020

Some quick changes to resolve the issues #1, #2 and #4 that I created earlier.

@FalseHonesty
Copy link

Hey man, it would be great if you could squash your commits to not add unnecessary commits. It seems to me that the two commits for changing the Example can be squashed into one. As for the other two commits, the content looks good, but if you could add the text Resolves #x to the commit message bodies for those two (With the x obviously being replaced for the issue it closes), that would be great :)

@solonovamax
Copy link
Author

I don't normally do things like squashing commits so I'll have to google around for that, but I can try to do that right now.

@FalseHonesty
Copy link

Not a problem, let me know if I can help, it's good practice to learn early so you know for later :)

@solonovamax
Copy link
Author

I also forgot to update the data.json file, so I did that here.

@solonovamax
Copy link
Author

But I've haven't used github to contribute to projects that much, so I don't know much of the etiquette related to it. I just kind of made a commit when I added something.

@FalseHonesty
Copy link

I totally understand, a lot of projects don't really mind how they do commits, but I enjoy keeping a clean commit tree, and I also enjoy seeing people learn git more in-depth than just adding, committing, and pushing, so I hope you have added this skill to your repertoire. @Sk1er will still have to review this PR, but the commit structure looks great to me.

@FalseHonesty FalseHonesty requested a review from Sk1er August 6, 2020 20:04
@solonovamax solonovamax changed the title Resolving issues #1 and #2 Resolving issues #1 and #2 and #4 Aug 6, 2020
@solonovamax solonovamax changed the title Resolving issues #1 and #2 and #4 Resolving issues #1, #2, and #4 Aug 7, 2020
@solonovamax
Copy link
Author

solonovamax commented Sep 22, 2020

I'm lowkey kinda waiting on this to be approved so I can make a pull request for this lmao (it says 4 commits ahead, but it's only 1 commit ahead of this pull request)

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.

2 participants