Skip to content
This repository has been archived by the owner on Jan 26, 2018. It is now read-only.

minimal hook #5

Merged
merged 57 commits into from
Feb 9, 2017
Merged

minimal hook #5

merged 57 commits into from
Feb 9, 2017

Conversation

orthagonal
Copy link
Collaborator

No description provided.

scripts: process.cwd()
scripts: process.cwd(),
githubRoute: '/',
simpleRoute: '/simple',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't look like you are checking this


module.exports = (dataToProcess, options, callback) => {
async.autoInject({
beforeHooks: done => runFirstExistingScript([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really hard to read, can you put { } around?

}
// todo: does this pass any params to the script?
runshell(existingScript, {
env: process.env,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to set REPO, BRANCH, USER on env. I believe new version of runshell already uses process.env


module.exports = (request, response, options) => {
SimpleSchema.validate(request.payload, (err, result) => {
if (err || result.length !== 4) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic seems wrong... if err?

@jgallen23 jgallen23 merged commit 600f6af into master Feb 9, 2017
@jgallen23 jgallen23 deleted the minimalHook branch February 9, 2017 05:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants