search

HackerHappyHour / impact

Org friendly chat
MIT License
1 stars 0 forks source link

Setup hapi server #13

Closed LongLiveCHIEF closed 8 years ago

LongLiveCHIEF commented 8 years ago

closes #7

geekytime commented 8 years ago

Can we turn on linting before we merge any JS code?

LongLiveCHIEF commented 8 years ago

Can we turn on linting before we merge any JS code?

Sure. That would be separate PR though, I don't want to mix up the scope of this one.

geekytime commented 8 years ago

Can we turn on linting before we merge any JS code?

Sure. That would be separate PR though, I don't want to mix up the scope of this one.

I just opened #16

geekytime commented 8 years ago

So normally I'd recommend using nconf for stuff like this, but it's been annoying me lately. Do you know of any other tiered config libs?

LongLiveCHIEF commented 8 years ago

There's node-config, but honestly, I want to really give the method I'm already using a shot, because... it's native to node, and has all the capabilities of heirarchical configs we've used in the past.

geekytime commented 8 years ago

No, man. It's native to npm. :frowning:

geekytime commented 8 years ago

Putting our config in there would be like putting it in the .eslintrc file, or the .babelrc file. .npmrc is for npm config, not app config.

geekytime commented 8 years ago

There's node-config, but honestly, I want to really give the method I'm already using a shot, because... it's native to node, and has all the capabilities of heirarchical configs we've used in the past.

How about Seraphim?

Edit: Never mind. That looks stupid. It's been highly recommended in a few places, but I kind of super-hate it.

LongLiveCHIEF commented 8 years ago

How about this, accept it the way it is, and open an issue to select and integrate a better config toolset?

geekytime commented 8 years ago

This is the one I was looking for, not seraphim: convict.

geekytime commented 8 years ago

That right there is the guy to try.

geekytime commented 8 years ago

not goofy, just git-fu.

This totally borks up this merge request. Now we have to re-review code we've already merged. Don't do this. It's bad.

geekytime commented 8 years ago

not goofy, just git-fu.

This totally borks up this merge request. Now we have to re-review code we've already merged. Don't do this. It's bad.

By rebasing you confused git about what commits are in your branch. It doesn't know that some of this code is already in master. You need to leave those commits in the merge commit alone to avoid this.

geekytime commented 8 years ago

Why don't we slow down a bit, back up, and see if we can get convict working first. And maybe add the rethink API?

geekytime commented 8 years ago

A web server with nothing to read or write to doesn't really help us much, anyway.

LongLiveCHIEF commented 8 years ago

By rebasing you confused git about what commits are in your branch. It doesn't know that some of this code is already in master. You need to leave those commits in the merge commit alone to avoid this.

It wasn't the rebase that borked it, it was the fact that the refs changed for the branch when I went to push. You'll notice that git doesn't believe there are any conflicts, because when I rebased, it moved all my commits onto the HEAD ref that already included this .eslintrc.

We won't need to re-review anything, because it will merge the file cleanly.... we just wound up with an extra merge commit in the history.

I shouldn't have rebased, but only because I already had refs pushed to a remote.

LongLiveCHIEF commented 8 years ago

Dude seriously... you say one thing about how we should work, and then leave a bunch of comments that oppose that.

I want to test other stuff from the test env setup, and I need to have the basic hapi server in there to do that. I can't fork off of work you might be doing with this stuff to do that stuff, because you'll have a cow.. so I'm putting the most basic stuff I need into a PR, in order to complete the actual work I'm trying to do.

LongLiveCHIEF commented 8 years ago

Why don't we slow down a bit, back up, and see if we can get convict working first. And maybe add the rethink API?

Because in order to do that, I need you to give a thumbsup to #5.

geekytime commented 8 years ago

5 didn't work right on my machine. I can't give it a thumbsup, but I did leave some more comments over there.

LongLiveCHIEF commented 8 years ago

Did you try it again after I fixed the bug?

geekytime commented 8 years ago

Let's discuss it over there.

LongLiveCHIEF commented 8 years ago

I'm going to close this PR, join it with the hapi-setup one, and add a connection from hapi to both rethink and the ldap server, then submit that as a PR, including better docs for how to use the local test servers with or without vagrant.

Cool?

geekytime commented 8 years ago

Okey doke.