NodeJS support #183
NodeJS support #183
Conversation
michelvocks
added
the
Needs Work
michelvocks
force-pushed
the
nodejs_support
branch
from
8d3aa2c
to
2ddc0b3
Compare
michelvocks
added
needs review
Ready To Merge
Codecov Report
@@ Coverage Diff @@
## master #183 +/- ##
==========================================
+ Coverage 62.36% 62.59% +0.22%
==========================================
Files 47 48 +1
Lines 3829 3914 +85
==========================================
+ Hits 2388 2450 +62
- Misses 1051 1069 +18
- Partials 390 395 +5
Continue to review full report at Codecov.
|
| gaia.Cfg.Logger.Error("error", string(out[:])) | ||
| return fmt.Errorf("cannot install ruby gem: %s", string(out[:])) | ||
| } | ||
| case gaia.PTypeNodeJS: |
There was a problem hiding this comment.
At some point we'll need to refactor this into something sensible like a plugin type factory of some sorts, because this switch case is getting ridicules. :D
At the very least we should extract the individual legs into small functions. That has the added benefit of noticing commonalities between these legs better.
There was a problem hiding this comment.
Yeah, I completely agree.
I personally think for now it's "fine" since we have two or three switches which you need to modify when you add a new language so that's overall not much but for the future, a lot of refactoring will be required!
But that is a problem for the future Michel (😅)
|
Going to give this a spin now. |
| // PrepareEnvironment prepares the environment before we start the build process. | ||
| func (b *BuildPipelineNodeJS) PrepareEnvironment(p *gaia.CreatePipeline) error { | ||
| // create uuid for destination folder | ||
| uuid := uuid.Must(uuid.NewV4(), nil) |
There was a problem hiding this comment.
Shouldn't we generalise to use GenerateRandomUUIDV5?
There was a problem hiding this comment.
Good idea. We can do that after merge and then for all builders?
| } | ||
|
|
||
| // Unpack it | ||
| cmd := exec.Command(path, "-xzvf", p.ExecPath, "-C", tmpFolder) |
There was a problem hiding this comment.
I'm wondering if we really need the -v flag? I don't think we really need this to be verbose, don't we?
There was a problem hiding this comment.
If the command fails, the output is displayed in the logs. This could be helpful.
Fixes #183
Docs PR: gaia-pipeline/docs#12