These are chat archives for deployd/contributors

21st
Aug 2015
Eric Fong
@ericfong
Aug 21 2015 16:01
Reviewing PR, Is #637 and #639 want to solve same problem?
Rares Golea
@rgolea
Aug 21 2015 16:02
It is… I haven’t seen that one in time...
I have some addtional stuff there… not just the require() problem
Eric Fong
@ericfong
Aug 21 2015 16:04
If we want to solve the require in a good way, we should ensure the require can require
  1. relative (relative to the event file) ** need test on that
  2. absolute path (should be ok)
  3. module path (should be ok)
I think it is better to put require in one PR. @n321203 have something like context checking
Rares Golea
@rgolea
Aug 21 2015 16:07
If @321203 solves all of those problems, go ahead and use that PR
Rares Golea
@rgolea
Aug 21 2015 16:12
the idea was to solve that problem :D
Eric Fong
@ericfong
Aug 21 2015 16:12
Actually, hope PR creator can write some test on feature like the require, in order to produce better code base. (Hard to find contributor to write test cases).
Originally, require is a "leak out" feature, so no test on that.
If we want to make it better. Then, we better test on (I suggest)
  • relative (relative to the event file?)
  • absolute path
  • module path
Actually, I think your PR is may be able to handle case 1
Rares Golea
@rgolea
Aug 21 2015 16:14
but it handles from the absolute path I think...
Eric Fong
@ericfong
Aug 21 2015 16:16

What I expect when I write

require('./../myfile.js')

in /resources/user/get.js

I think that it can require /resources/myfile.js

Rares Golea
@rgolea
Aug 21 2015 16:17
I’m pretty much sure that if you want to require a file from resources you might have to use requireLib(‘/resources/myfile.js’) at least in my PR
Eric Fong
@ericfong
Aug 21 2015 16:19
Yes, but 'requireLib' seems add a one more interface to it. I am afraid people will be confused.
And 'require('./../myfile.js')' will require a wrong file. So, I prefer reuse the require interface and make it works in the expected way (We can discuss what the expected way it, and how to resolve relative path, but better have some handle on that).
Rares Golea
@rgolea
Aug 21 2015 16:21
Yeah… I know… had some problems with the require thing… maybe we can add more to that… from ctx
Eric Fong
@ericfong
Aug 21 2015 16:22
For the Script object, there is a 'this.path' may be we can use that to get relative path from there
Rares Golea
@rgolea
Aug 21 2015 16:24
maybe I’ve got a way to fix that but I will try it tomorrow night
:)
or on sunday
Eric Fong
@ericfong
Aug 21 2015 16:24
Yes, need to sleep now too. see u
Rares Golea
@rgolea
Aug 21 2015 16:24
so… require() should require from the resource folder, right?
next to the event?
or from the folder itself?
Eric Fong
@ericfong
Aug 21 2015 16:26

For me,

Inside '/resources/user/get.js'
require('../myfile.js')
Will take '/resources/myfile.js'

Inside '/resources/user/get.js'
require('./common.js')
Will take '/resources/user/common.js'

May be @/all can have diff views on that
Rares Golea
@rgolea
Aug 21 2015 16:27
I will do it like that and send it back… expect something on sunday
:)
Eric Fong
@ericfong
Aug 21 2015 16:28
Thx a lot
Rares Golea
@rgolea
Aug 21 2015 16:28
btw… I edited the Readme.md on dpd-email
;)
Eric Fong
@ericfong
Aug 21 2015 16:28
Will work on that too
Rares Golea
@rgolea
Aug 21 2015 16:29
ok… cool! cya