Grow your CSS skills. Land your dream job.

#117: Let’s Attempt To Do a “Pull Request”

I've never in my life submitted a "Pull Request" on GitHub. I wanted to give it a shot, so this video is capturing the moment of me attempting to figure it out. Fair warning, this isn't a succinct, quick tutorial. This is me thinking to myself and struggling through it all.

My idea was that I wanted to put FitVids.js onto cdnjs so people can link it up through there. Their process for doing that is by forking their GitHub repo, follow their instructions for adding a new library, and submit a pull request.

So we do it!

Things I struggled with:

  • I didn't understand if a pull request was a "Git" thing or a "GitHub" thing. I'm still not 100% sure but it seems like it's a GitHub thing, as we did it 100% through GitHub.com.
  • I didn't understand what exactly composes a "pull request" - I thought it would be more like you move a specific commit over to another repo. We learned that it's actually the entire repo that you move together (all commits made since your fork).
  • One of their requirements was that it pass the "npm test" thing locally. It didn't seem to pass that even with an unchanged fork of the repo. Ultimately I did screw up the first pull request also with an incorrect file name. This failed the "Travis build" on GitHub.com, which I also don't understand (is it the same as the npm test?), but after fixing the file name the second request worked.

As a follow up, the owners of the cdnjs repo asked that I include the non-minified version in the repo and add a "tag" to the FitVids repo with the correct versions. So I had to learn another brand new thing, but ultimately got it done.

The Pull Request

Comments

  1. Hi Chris,
    Good to see more people getting into Git and GitHub.

    Pull requests are a GitHub thing however most repository hosts with a GUI will use their own terminology for it.

    Also when you submit your new branch with the changes to your fork of the repository, a button should appear in the base of the repository (at the directory viewer) but the way you did it is also acceptable.

    The idea of pull requests is that you are requesting a set of changes via commits that you would like to have changed so using the branching model mentioned above makes more sense to use.

  2. Matt
    Permalink to comment#

    “There’s only one way to learn this stuff, and it’s by screwing it up, getting yelled at by someone, then you fix it. Then you see someone else being dumb and you yell at them. And that’s how this works”

    This needs to be a poster.

  3. Permalink to comment#

    Hi Chris, i’ve noticed that you use a nice theme for your sublime editor with a cool animation of open/close folder on the sidebar. Would you be nice to tell me the name of the theme??
    Thanks

  4. Permalink to comment#

    Hey Chris, “npm test” failed because the dependencies for the test weren’t installed (vows). The package.json (for the tests) lists those dependencies. However, they’re not automatically installed. You’d have to shell to the tests dir and do an “npm install” first. – provided you have Node.js and npm installed.

    You have to do this because dependencies aren’t normally packaged with the rest of the repo because of versioning and repo bloating.

    The whole tests thing is a great tool, but not easy or useful unless you’re a Node.js developer. Should send cdnjs a pull request to add documentation for running tests.

  5. A pull request in itself is not a hard thing, as you have seen. Not in GitHub at least. Maybe this case with cdnjs was kinda tricky, but in general, it’s not hard to achieve.

    Good job, Chris.

  6. Node has 2 different types of dependencies: “production” and “dev”. To run npm test you want to install the dev dependencies with npm install --dev (which also installs prod dependencies btw).

    I actually got an error while installing modules from npm for cdnjs, but npm test ran ok, so I’m calling that good.

    It seems like others have explained your other troubles. Don’t get discouraged, everyone was where you are now.

  7. This will come in very handy when I finally get around to doing a pull request. I like watching your videos because they aren’t fast and cut to perfection, we actually get to see your thought processes behind a new thing, which I think is much more helpful even if sometimes it does leave us yelling at the screen! That said it’s easy to say that as a third party, it’s a lot harder to figure stuff out by yourself the first time. Great job, thanks Chris!

  8. Chris, this was awesomely helpful. I’m still somewhat new to git and have been slowly introducing my team to all its particulars. Keep the videos coming!

  9. Hey Chris, Just watched the video and wanted to give you some Sublime Text 2 tips! There is a great package for Sublime Text called “SideBarEnhancements”. It allows you to right click and give you the option to rename the file. It also has a ton of other different features. You can find it here

    https://github.com/titoBouzout/SideBarEnhancements.

    Another great package I found that might help you out is a JS minifier. I’m sure you take care of minifying your JS through WordPress but if you wanted to do it right through Sublime Text you could install this package..

    https://github.com/adampresley/sublime-js-minify

  10. There definitely needs to be more git video like this out there. Thanks for sharing, Chris!

  11. Permalink to comment#

    Looks like you made it in.

    //cdnjs.cloudflare.com/ajax/libs/fitvids/1.0.1/jquery.fitvids.min.js

  12. Jonathan Kupcho
    Permalink to comment#

    npm install -g vows should allow your npm test to pass.

    vows is a testing framework for nodejs

    The -g argument allows you to install it “globally” allowing you to type ‘vows [arguments]’ at the shell.

Leave a Comment

Posting Code

Markdown is supported in the comment area, so you can write inline code in backticks like `this` or multiline blocks of code in in triple backtick fences like ```this```. You don't need to escape code in backticks, Markdown does that for you.

Sadly, it's kind of broken. WordPress only accepts a subset of HTML in comments, which makes sense, because certainly some HTML can't be allowed, like <script> tags. But this stripping happens before the comment is processed by Markdown (via Jetpack). It seems to me that would be reversed, because after Markdown processes code in backticks, it's escaped, thus safe. If you think you can fix this issue, get in touch!

If you need to make sure the code (typically HTML) you post absolutely posts correctly, escape it and put it within <pre><code> tags.

Current ye@r *

*May or may not contain any actual "CSS" or "Tricks".