search

junior-dev-struggle-bus / juniordevstrugglebus

Website for the Junior Dev Struggle Bus Meetup
https://www.juniordevstrugglebus.com
MIT License
9 stars 18 forks source link

Removed React favicon #14

Closed bhurstGH closed 5 years ago

bhurstGH commented 5 years ago

Simple pull request test, deleting unused favicon

bhurstGH commented 5 years ago

Did a lot of clean up of errors, warnings, accessibility, etc.

bhurstGH commented 5 years ago

Rewrote events pages. Uses Meetup API now. Also implemented a button to hide the normal biweekly meetings so people could filter through to the more "special" events like Michael and Daniel's. Some clean up of old files and reimplementation/new implementation of loading state needed.

nspilman commented 5 years ago

My f63king man. Looks great. One note - the pull request comment is currently "Removed Favicon", and you did wahahaaay more than that, so I've renamed it to "Major website cleanup and Meetup,com API integration." -Nate

phoenixcoder commented 5 years ago

If you ever wanted to combine commits so you can give a better summary of what happened, you can run:

git rebase -i

This is an interactive rebase that'll take you through the commit history, and let you smush them together with a new commit message. You normally do this at the end when your stuff has been reviewed and approved.

bhurstGH commented 5 years ago

I'll definitely have to dig into those particulars. I was wonder, as I went along, if there was a better way to handle how I was going about it.

I made that first Pull Request, when I removed the favicon, just as a test. My very first PR. It continued to add any further commits/changes on to that original one. Was there something I should have done to make separate pull requests or can you only have one open at a time for a project?

phoenixcoder commented 5 years ago

To make separate pull requests, you have to put the commits on separate branches, and initiate the request from each of those branches. The repo owner @nspilman can then merge them into the repo in a single commit or rebase them for you.

bhurstGH commented 5 years ago

Ah ha -- that makes sense. I was working off of feature branches, which I merged into a dev branch. Once I was sure all of that was working, I would merge dev to master and make the pull request with that.

Noted for next time!

bhurstGH commented 5 years ago

@phoenixcoder Does all of this generally sound correct:

Fork original JDSB Project to your own GitHub Clone your personal fork locally Your fork is origin Set original JDSB Project as upstream (Potentially fetch from upstream if someone makes changes pertinent to what you are working on? Are there automatic notifications for this, or do you have to check manually? Periodically? Before PR?)

Gaston has been doing some work on the site as well and we were talking about the proper workflow on Monday.

phoenixcoder commented 5 years ago

That sounds generally right, if you just want one PR. If someone pushes changes before you push yours, you'll have to

  1. Pull the changes into your local first.
  2. If there's a conflict, perform a manual merge.
  3. Test changes with the new commit(s).
  4. Submit a new PR.
phoenixcoder commented 5 years ago

The workflow is generally: whoever submits working changes first, wins! Then either the submitter or the merger/repo owner will perform the conflict merges, if any.

bhurstGH commented 5 years ago

One more for good measure before I let you get back to your day!

Granted, I made my first PR as a test using master branch, so all my subsequent changes to master were added on to this original PR. You've already explained that I should have made PRs from separate branches which is good to know.

What about canceling/closing a PR if you realize you've done something like I did, or perhaps realized a bug of some kind after you've already made the PR. Is that a relatively painless process of simply closing the PR from this page before it has been accepted by the repo owner, or are there unforeseen consequences?

Covering some seemingly simple bases because all of this will probably make it into some kind of "How to contribute" guide, and assuming minimal knowledge from the user is best if you don't know your audience. Alleviating fear and apprehension towards participating is desirable, and having answers before a question like "Eek, I screwed up! How do I fix it?" is even asked goes a long way.

phoenixcoder commented 5 years ago

Yeah, you can simply close the PR and make a new pull request. Github tracks the pull request based on the hashcode of the commit. So, if you:

  1. Update the commit, but the hashcode does not change. The subsequent PR will just update your old PR.
  2. Create a brand new commit. The PR will be brand new.
  3. You can also delete the branch your PR was on, and go to a new branch entirely, after you've closed your PR.
bhurstGH commented 5 years ago

You da best! Thanks, chief.