Clean up and Github API

[msied]

...way from app.js so reading this thing is easier

[markbao]

👍

[msied]

Maybe we should just use the github API to read who the contributors to the master branch are, and make that the list? Then we can make the site github-login only.

[pents90]

@msied using the Github API to track contributors is a great idea.

[pents90]

Getting this error, I think passport needs to be passed into config/index.js, ideas?

ReferenceError: passport is not defined
    at module.exports (/Users/username/code/codepile/routes/index.js:59:28)
    at Object.<anonymous> (/Users/username/code/codepile/app.js:30:20)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:902:3

[msied]

My bad. Just pushed the fix.

[msied]

Sorry about that. Something else was wrong--but now it works and runs locally without a hitch.

[msied]

UPDATE: added github api use for authentication. On each login attempt, the userlist is queried from repo contributors of this repo.

[megamattron]

This is awesome, thanks. Do you know what counts as a contributor in terms of the api? Is it only people who have code in the tree via pull request or direct commit?

[msied]

It's just people who have had their stuff merged to master, it looks like. https://api.github.com/repos/larvalabs/pullup/contributors

[megamattron]

I'm split on whether to use the API for 2 reasons:

• How do you log in on your local machine before you're a contributor?
• There's a nice simplicity to adding your username to an array as part of your change. Although it does stink that I'm merging so many conflicts.

Not sure overall.

[msied]

You change the instructions. Define the pre-query array with one member
(dev github name) instead of empty, before populating it from the API. Same
setup cost as adding to the userlist config.

On Friday, February 7, 2014, Matt Hall notifications@github.com wrote:

I'm split on whether to use the API for 2 reasons:

• How do you log in on your local machine before you're a contributor?
• There's a nice simplicity to adding your username to an array as
  part of your change. Although it does stink that I'm merging so many
  conflicts.

Not sure overall.

Reply to this email directly or view it on GitHubhttps://github.com//pull/11#issuecomment-34485330
.

[msied]

Or you just look for process.env.PORT during auth. If it doesn't exist (loosely implying local dev [will need a more accurate indicator]), auth is bypassed and login allowed.

[megamattron]

Having gone through a bunch of merges now, there's another issues that's coming up. There are people that I've taken code from both of their PRs, made one commit and added both their names and pushed that. So they don't appear in the official contributers, but they're users. The github api scheme would force me to accept a PR to allow them access. Maybe that's not a big deal, they could just do a simple unrelated PR, not sure.

[msied]

That's the beauty of the github auth solution. You do automatic PR merges
(not manual as you need to do for the faulty userlist solution), so they're
going to be added as contributors. If they made changes that need to be
merged manually, you tell them to fix them until they're in a state to be
automatically merged.

On Fri, Feb 7, 2014 at 12:47 PM, Matt Hall notifications@github.com wrote:

Having gone through a bunch of merges now, there's another issues that's
coming up. There are people that I've taken code from both of their PRs,
made one commit and added both their names and pushed that. So they don't
appear in the official contributers, but they're users. The github api
scheme would force me to accept a PR to allow them access. Maybe that's not
a big deal, they could just do a simple unrelated PR, not sure.

Reply to this email directly or view it on GitHubhttps://github.com//pull/11#issuecomment-34500998
.

[treygriffith]

I created an issue (#67) for dealing with this feature. For local dev, couldn't you just reference your own GitHub fork of the repo? Or would you not be listed as a contributor until you pushed code to your repo?

I think using process.env.PORT is a bit hacky, it would be better to have a dedicated environment variable denoting the environment (production vs dev).

[msied]

@treygriffith I agree about the hackyness of that suggestion--I just wanted to address @megamattron 's concerns quickly to demonstrate the non-issue. I'm still not a merged/auth'd user.

A process.env.PRODUCTION = true would be ideal.

[megamattron]

Yeah, I'm definitely leaning towards the API route now, especially after doing a ton of manual merges on the userlist file :) Let me give this PR a shot and see if I can get it working.

[treygriffith]

@megamattron The people whose PR's you didn't directly accept may have to resubmit a new PR to get access after this PR is merged, but that's only a handful of users, and access to Pullup isn't exactly mission critical yet.

[megamattron]

@msied I just realized you never did get access while we were figuring this out, I'm going to add you to the user list because of the contribution even though I'm not quite sure what I'm doing with this PR yet. You should have access shortly.

[megamattron]

The alphabetical userlist has solved the merging problem for the most part, but I'd love to use the github API for user contribution stats, anyone interesting in retasking this for that purpose?