Use precise revision hash length #28
Conversation
lexmarin
commented
Jan 13, 2017
lexmarin
commented
- ran into problem using incorrect revision hash length
- using precise revision hash length now
lexmarin
force-pushed
the
feature/rev-hash-length
branch
from
29f302f
to
1069e27
Compare
lexmarin
force-pushed
the
feature/rev-hash-length
branch
2 times, most recently
from
6428d69
to
0109834
Compare
| request.post(config.notifyWebHook).send(payload).end(); | ||
| if (config.notifyWebHook) { | ||
| request.post(config.notifyWebHook).send(payload).end(); | ||
| } |
| var DEFAULT_CONFIG_ABBREV = 7; | ||
|
|
||
| var path = require('path'); | ||
| var config = require(path.join(process.cwd(), CONFIG_FILENAME)) |
There was a problem hiding this comment.
There are more places where the config could be stored. See https://github.com/productboard/webpack-deploy/pull/28/files#diff-1bf37fe770044c540521fd30ab26dec4R19
We could just export a function to get the config file.
| var argv = require('yargs').string('env').argv; | ||
|
|
||
| var env = argv.env; | ||
| var abbrev = (config['git'][env] && config['git'][env].abbrev) || DEFAULT_CONFIG_ABBREV; |
There was a problem hiding this comment.
Also should check for config['git'].abbrev
| @@ -54,7 +54,8 @@ case "$ENV" in | |||
| exit 1 | |||
| esac | |||
|
|
|||
| COMMIT=`git rev-parse --short HEAD` | |||
| ABBREV_LENGTH=`node ${DIRNAME}/config-abbrev --env=${ENV}` | |||
There was a problem hiding this comment.
As you said, I also don't think this is the ideal approach. Nevertheless it gets the job done and that's what's important here. We need to revisit this whole JS/Bash interop soon anyway.
lexmarin
force-pushed
the
feature/rev-hash-length
branch
2 times, most recently
from
41ffeab
to
478f3e3
Compare
|
@LeZuse I've incorporated changes and refactored requiring deploy config, please take a look at this. |
| return require(filename); | ||
| } | ||
| function requireConfig(silent) { | ||
| var filename = CONFIG_PATHS.filter(function(configPath) { |
There was a problem hiding this comment.
You can use find instead of filter()[0]
| var config = getConfigFor('git', true); | ||
| var abbrev = (config && config.abbrev) || DEFAULT_CONFIG_ABBREV; | ||
|
|
||
| process.stdout.write(abbrev.toString()); |
There was a problem hiding this comment.
The bin/ folder is meant for executables. I would suggest to move this to utils.js, tasks/ or some new folder.
|
@marinale A few simple changes and we should be ready to merge. |
lexmarin
force-pushed
the
feature/rev-hash-length
branch
from
478f3e3
to
63a1d2b
Compare
|
@LeZuse Changes incorporated, installed and tested this particular commit in other repo and it works! Thanks for your review! |
