Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial WIP for procbots. #5

Merged
merged 8 commits into from
Feb 2, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions build/versionbot.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion build/versionbot.js.map

Large diffs are not rendered by default.

35 changes: 16 additions & 19 deletions gulpfile.js
Expand Up @@ -6,38 +6,35 @@ const tslint = require('gulp-tslint');
const tsProject = typescript.createProject('tsconfig.json');

const OPTIONS = {
header: true,
files: {
typescript: [ 'lib/**/*.ts' ],
declarations: [ 'lib/**/*.d.ts' ]
},
base: 'lib'
header: true,
files: {
typescript: [ 'lib/**/*.ts' ],
declarations: [ 'lib/**/*.d.ts' ]
},
base: 'lib'
}

// Compile the TS sources
gulp.task('typescript', () => {
gulp.src(OPTIONS.files.typescript)
.pipe(sourcemaps.init())
.pipe(tsProject()).on('error', gutil.log)
.pipe(sourcemaps.write('./', { includeContent: true,
sourceRoot: '../lib',
// There is currently an issue where not doing this won't generate
// correctly pathed mapfiles.
mapSources: (pathFile) => { return pathFile; }
}))
.pipe(gulp.dest('build/'));
gulp.src(OPTIONS.files.typescript)
.pipe(sourcemaps.init())
.pipe(tsProject()).on('error', gutil.log)
.pipe(sourcemaps.write('./', { includeContent: true,
sourceRoot: '../lib'
}))
.pipe(gulp.dest('build/'));
});

// Copy any pre-defined declarations
gulp.task('copydecs', () => {
gulp.src(OPTIONS.files.declarations)
.pipe(gulp.dest('build/'));
gulp.src(OPTIONS.files.declarations)
.pipe(gulp.dest('build/'));
});

gulp.task('tslint', () =>
gulp.src(OPTIONS.files.typescript)
.pipe(tslint({
configuration: 'tslint.json',
configuration: 'tslint.json',
formatter: 'prose'
}))
.pipe(tslint.report())
Expand Down
10 changes: 5 additions & 5 deletions lib/versionbot.ts
Expand Up @@ -25,14 +25,14 @@ import * as GithubBotApiTypes from './githubapi-types';
import * as GithubBot from './githubbot';
import { GithubAction, GithubActionRegister } from './githubbot-types';
import * as ProcBot from './procbot';
import { mkdir, track } from 'temp';

// Declare correct bindings for requires without typings.
// Exec technically has a binding because of it's Node typings, but promisify doesn't include
// the optional object (we need for CWD).
const fsReadFile = Promise.promisify(FS.readFile);
// the optional object (we need for CWD). So we need to special case it.
const exec: (command: string, options?: any) => Promise<{}> = Promise.promisify(ChildProcess.exec);
const tempMkdir: (dir: string) => Promise<{}> = Promise.promisify(require('temp').mkdir);
const tempCleanup = Promise.promisify(require('temp').cleanup);
const fsReadFile = Promise.promisify(FS.readFile);
const tempMkdir = Promise.promisify(mkdir);
const tempCleanup = Promise.promisify(track);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can promisify track if you like, but looking at the docs it doesn't take a callback or return anything at all, so I probably wouldn't 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point.


// Specific to VersionBot
interface FileMapping {
Expand Down
127 changes: 63 additions & 64 deletions package.json
@@ -1,66 +1,65 @@
{
"name": "resin-procbots",
"version": "0.0.1",
"description": "Process robots for automating CI pipelines",
"homepage": "https://github.com/resin-io-modules/resin-procbots#readme",
"preferGlobal": true,
"types": "build/**/*.d.ts",
"keywords": [
"resin",
"process"
],
"author": "Heds Simons <hedley@resin.io>",
"license": "Apache-2.0",
"main": "./build/app.js",
"bin": {
"procbots": "./bin/procbots"
},
"repository": {
"type": "git",
"url": "git+https://github.com/resin-io-modules/resin-procbots.git"
},
"bugs": {
"url": "https://github.com/resin-io-modules/resin-procbots/issues"
},
"scripts": {
"build": "gulp build",
"start": "node ./bin/procbots -b versionbot"
},
"devDependencies": {
"@types/bluebird-global": "^3.0.0",
"@types/body-parser": "0.0.33",
"@types/express": "^4.0.34",
"@types/github": "^0.0.0",
"@types/jsonwebtoken": "^7.2.0",
"@types/lodash": "^4.14.43",
"@types/node": "^6.0.48",
"@types/node-getopt": "^0.2.29",
"@types/request-promise": "^4.1.33",
"@types/request": "^0.0.39",
"gulp": "^3.9.1",
"gulp-util": "^3.0.7",
"gulp-typescript": "^3.1.4",
"gulp-tslint": "^7.0.1",
"gulp-sourcemaps": "^2.3.1",
"jsdoc": "^3.4.3",
"typescript": "^2.1.5",
"tslint": "^4.4.2"
},
"dependencies": {
"bluebird": "^3.0.35",
"body-parser": "^1.9.2",
"express": "^4.0.34",
"github": "^8.0.0",
"github-webhook-handler": "^0.6.0",
"hmac": "^1.0.1",
"jsonwebtoken": "^7.2.1",
"lodash": "^4.17.2",
"node-getopt": "^0.2.3",
"mkdirp": "^0.5.1",
"request-promise": "^4.1.1",
"request": "^2.79.0",
"rmdir": "^1.2.0",
"temp": "^0.8.3",
"versionist": "^2.8.0"
}
"name": "resin-procbots",
"version": "0.0.1",
"description": "Process robots for automating CI pipelines",
"homepage": "https://github.com/resin-io-modules/resin-procbots#readme",
"preferGlobal": true,
"keywords": [
"resin",
"process"
],
"author": "Heds Simons <hedley@resin.io>",
"license": "Apache-2.0",
"main": "./build/app.js",
"bin": {
"procbots": "./bin/procbots"
},
"repository": {
"type": "git",
"url": "git+https://github.com/resin-io-modules/resin-procbots.git"
},
"bugs": {
"url": "https://github.com/resin-io-modules/resin-procbots/issues"
},
"scripts": {
"build": "gulp build",
"start": "node ./bin/procbots -b versionbot"
},
"devDependencies": {
"@types/bluebird-global": "^3.0.0",
"@types/body-parser": "0.0.33",
"@types/express": "^4.0.34",
"@types/github": "^0.0.0",
"@types/jsonwebtoken": "^7.2.0",
"@types/lodash": "^4.14.43",
"@types/node": "^6.0.48",
"@types/node-getopt": "^0.2.29",
"@types/request-promise": "^4.1.33",
"@types/request": "^0.0.39",
"gulp": "^3.9.1",
"gulp-util": "^3.0.7",
"gulp-typescript": "^3.1.4",
"gulp-tslint": "^7.0.1",
"gulp-sourcemaps": "^2.3.1",
"jsdoc": "^3.4.3",
"typescript": "^2.1.5",
"tslint": "^4.4.2"
},
"dependencies": {
"bluebird": "^3.0.35",
"body-parser": "^1.9.2",
"express": "^4.0.34",
"github": "^8.0.0",
"github-webhook-handler": "^0.6.0",
"hmac": "^1.0.1",
"jsonwebtoken": "^7.2.1",
"lodash": "^4.17.2",
"node-getopt": "^0.2.3",
"mkdirp": "^0.5.1",
"request-promise": "^4.1.1",
"request": "^2.79.0",
"rmdir": "^1.2.0",
"temp": "^0.8.3",
"versionist": "^2.8.0"
}
}
27 changes: 14 additions & 13 deletions tsconfig.json
@@ -1,15 +1,16 @@
{
"compilerOptions": {
"module": "commonjs",
"declaration": true,
"noImplicitAny": true,
"removeComments": true,
"preserveConstEnums": true,
"strictNullChecks": true,
"sourceMap": true,
"target": "es6",
"types": [ "bluebird-global" ],
"noUnusedParameters": true,
"noUnusedLocals": true
}
"compilerOptions": {
"module": "commonjs",
"declaration": true,
"noImplicitAny": true,
"removeComments": true,
"preserveConstEnums": true,
"strictNullChecks": true,
"sourceMap": true,
"target": "es6",
"baseUrl": "./typings",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised by baseUrl. I've never seen it before, and the docs say it exists mainly for AMD projects.

Is this so that require('temp') finds typings/temp.d.ts? In that case, I think you just need to ensure that the file is already included in the build process - i.e. add typings/*.d.ts to includes here or your gulp.files() argument above. The module shouldn't need to resolved correctly from its name because it's an ambient module declaration - it will define the module name you can load it by explicitly by itself, independent of its location, as long as it's in the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed from this thread, as I was originally attempting to use typeRoots.

Adding it to the gulp file appears to cause Visual Code to complain as it doesn't find them. I'll use includes if that's better than baseUrl.

"types": [ "bluebird-global" ],
"noUnusedParameters": true,
"noUnusedLocals": true
}
}
4 changes: 4 additions & 0 deletions typings/temp.d.ts
@@ -0,0 +1,4 @@
declare module 'temp' {
function track(): void;
function mkdir(prefix: string): (err: Error, dirPath: string) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

mkdir returns a callback? Seems unlikely - this should be the 2nd argument, and mkdir probably returns void.

This suggests the type you're getting when you promisify this isn't right either atm. Looks like the only reason you don't hit an implicit any is because the only place you call it you then immediately specify the next promise argument type.

Promisify does return the correct promisified type if the function you pass to promisify takes a callback as the last argument, but if it doesn't then it'll return a plain Function (i.e. callable, but takes and returns any). The types are little hacky but kind of interesting, if you're into that sort of thing: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/bluebird/index.d.ts#L390-L403. It means it works most of the time, but it's not totally foolproof. Implicit any should catch any substantial issues generally though.

}