ShaneK / Matador

Front-end web interface for Bull Job Manager
MIT License
98 stars 51 forks source link

require path question #6

Closed kfatehi closed 10 years ago

kfatehi commented 10 years ago

hi there. thanks for having built this. quick question though -- why are you doing your requires this way

updateInfo = require(process.cwd()+'/lib/updateInfo.js');

instead of, say, this way

updateInfo = require('../lib/updateInfo.js'); (depending of course where the file is)

I guess one has to choose whether do requires relative to an expected root (e.g., process.cwd()) versus relative to files. I think the latter is better.

Reason I think so, is what if I've required your Matador in another app, now all the paths are wrong.

I just want to know if there's something im unaware of as to why you did it this way?

thanks

kfatehi commented 10 years ago

Hey Shane I just want to let you know I've taken a lot of the core of your code, DRY'd it up, replaced Q for bluebird, ripped out pretty much all other dependencies (except lodash) and made this: https://github.com/keyvanfatehi/express-bull

that addresses backend. frontend coming soon, i'll update here again in a few mins once i'm done ripping that out of my app

kfatehi commented 10 years ago

Here is the frontend code https://github.com/keyvanfatehi/react-bull

Thanks for your hard work on Matador -- there was absolutely no way I could get such tightly integrable components for job management done this quickly otherwise.

kfatehi commented 10 years ago

Also, before I sign off for the night -- here's where I learned about Matador, and my last comment on there shows why I created the above 2 projects instead of just using Matador itself, in case you are curious https://github.com/OptimalBits/bull/issues/53

ShaneK commented 10 years ago

Looks like you've done quite a bit of work. Well done!

So, as for the require question: I was still relatively new to Node at the time I made Matador and for some reason I couldn't get relative paths to work (absolutely no idea why, I use them in my projects now and they work fine), so I used the terrible solution of process.cwd() to get around to non-working relative paths. I never considered that somebody might try to use Matador as part of another project, and in fact never really considered anybody else would use Matador. I was also in a rush to get Matador out as quickly as possible, it wasn't really made for the public, I made it for the company I work for and released it public because I thought others might find a use for it. Also because I really liked the name. So I'm sorry about the relative paths issue, Matador really does need quite a bit of refactoring, but the version my company uses is a fork off this version and has some pretty major changes (I used react components instead of knockout table bindings, I used long polling instead of refreshing the data every 2 seconds or so, I implemented Google Authentication that has an email address filter, etc.), and since it was originally made for my company anyway, I've kinda just been working on that version. I really need to get around to upgrading this version of it though.

Your express/react version is very nice. Well done. I'm sorry Matador didn't work out for you.

kfatehi commented 10 years ago

Thanks! No don't be sorry man. It DID work out. That's the only reason those other two components now exist. That's cool that you're using a React frontend now too. In fact, it sounds like maybe Matador could use a rewrite where it uses external components to recreate the monolithic app.