search

openknowledge-archive / datapackage-identifier-js

NodeJS library for handling Data Package Ident(ifier)s or Ident(ifier) Strings.
http://dataprotocols.org/data-package-identifier/
4 stars 1 forks source link

Allow Packages in GitHub Subfolders and Branches #2

Closed waylonflinn closed 8 years ago

waylonflinn commented 8 years ago

This patch correctly handles github urls that point to branches, subfolders and combinations of the two.

This feature more easily allows the scenario implemented in https://github.com/okfn/dpm/pull/51 and whose workaround is found in https://github.com/okfn/dpm/pull/52

rufuspollock commented 8 years ago

@waylonflinn great patch - i've wanted this kind of thing for a while 😄

Any chance you could "squash" these commits - it would make reviewing a lot easier.

waylonflinn commented 8 years ago

@rgrp glad you appreciate it. I'll combine a couple of those commits.

The 'Files Changed' tab is also handy for dealing with ambitious pull requests. :smile:

waylonflinn commented 8 years ago

Squashed! :space_invader:

There are still a few commits here. So, to help sort through them, I'll describe the general pattern.

The format is a TDD flow that matches the following:

Since there are two features the result is two pairs of commits. Squashing merged the extraneous noise commits into one of the semantically important ones.

waylonflinn commented 8 years ago

A bit more information about how it was accomplished might also help.

I made more extensive use of the node url module. After extracting the parts of the path into an array (already present in the previous code) I edited those parts as necessary (along with the hostname) then used the url module to reform them back into a valid URL (now pointing at github's raw server). In the process I also extracted information we care about (like branch and package name).

The first pair of commits implements most of that (and extracts the branch name). The second pair of commits is about correctly extracting the package name from branches.

Hope that makes it a bit easier to review! :book:

rufuspollock commented 8 years ago

@waylonflinn this is just awesome and big props for TDD approach.

rufuspollock commented 8 years ago

Reviewed and merged 👍