From daf0f472b3a37d3dc158c0e6bb5c76035bc67037 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sat, 11 Apr 2015 13:29:32 -0700 Subject: [PATCH] feat(build): enforce formatting of some files. Our style guide includes formatting conventions. Instead of wasting time in reviewing PRs discussing things like indenting, and to avoid later deltas to fix bad formatting in earlier commits, we want to enforce these in the build. The intent in this change is to fail the build as quickly as possible in travis, so those sending a PR immediately know they should run clang-format and update their commit. When running locally, we want users to know about formatting, but they may not want to act on it immediately, until they are done working. For this reason, it is only a warning outside of the continuous build. This is done by having a check-format task which should run on most local builds, and an enforce-format task only run by travis. --- Brocfile-dart.js | 4 +-- gulpfile.js | 18 +++++++++-- scripts/ci/build_js.sh | 2 +- tools/broccoli/broccoli-ts2dart.ts | 2 +- tools/broccoli/broccoli-writer.d.ts | 4 +-- tools/broccoli/traceur/index.ts | 2 +- tools/broccoli/ts2dart.d.ts | 2 +- tools/broccoli/typescript/index.ts | 46 ++++++++++++++--------------- 8 files changed, 46 insertions(+), 34 deletions(-) diff --git a/Brocfile-dart.js b/Brocfile-dart.js index 3bc9d4d33a..f2f220b5aa 100644 --- a/Brocfile-dart.js +++ b/Brocfile-dart.js @@ -6,9 +6,9 @@ var path = require('path'); // Transpile everything in 'modules'... var modulesTree = new Funnel('modules', { - include: ['**/*.js', '**/*.ts', '**/*.dart'], // .dart file available means don't translate. + include: ['**/*.js', '**/*.ts', '**/*.dart'], // .dart file available means don't translate. exclude: ['rtts_assert/**/*'], // ... except for the rtts_asserts (don't apply to Dart). - destDir: '/', // Remove the 'modules' prefix. + destDir: '/', // Remove the 'modules' prefix. }); // Transpile to dart. diff --git a/gulpfile.js b/gulpfile.js index e0463d0e19..8801d1908b 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -504,9 +504,22 @@ gulp.task('build/format.dart', rundartpackage(gulp, gulpPlugins, { args: CONFIG.formatDart.args })); +function doCheckFormat() { + return gulp.src(['Brocfile*.js', 'modules/**/*.ts', 'tools/**/*.ts', '!**/typings/**/*.d.ts']) + .pipe(format.checkFormat('file')); +} + gulp.task('check-format', function() { - return gulp.src(['Brocfile*.js', 'modules/**/*.ts', '!**/typings/**/*.d.ts']) - .pipe(format.checkFormat('file')); + return doCheckFormat().on('warning', function(e) { + console.log("NOTE: this will be promoted to an ERROR in the continuous build"); + }); +}); + +gulp.task('enforce-format', function() { + return doCheckFormat().on('warning', function(e) { + console.log("ERROR: Some files need formatting"); + process.exit(1); + }); }); // ------------ @@ -752,6 +765,7 @@ gulp.task('build.js.dev', function(done) { runSequence( 'broccoli.js.dev', 'build/checkCircularDependencies', + 'check-format', done ); }); diff --git a/scripts/ci/build_js.sh b/scripts/ci/build_js.sh index 5198cc870d..63c14b1a2e 100755 --- a/scripts/ci/build_js.sh +++ b/scripts/ci/build_js.sh @@ -8,4 +8,4 @@ SCRIPT_DIR=$(dirname $0) source $SCRIPT_DIR/env_dart.sh cd $SCRIPT_DIR/../.. -./node_modules/.bin/gulp build.js docs +./node_modules/.bin/gulp enforce-format build.js docs diff --git a/tools/broccoli/broccoli-ts2dart.ts b/tools/broccoli/broccoli-ts2dart.ts index 70fed135d2..28e098deaa 100644 --- a/tools/broccoli/broccoli-ts2dart.ts +++ b/tools/broccoli/broccoli-ts2dart.ts @@ -9,7 +9,7 @@ import path = require('path'); import ts2dart = require('ts2dart'); type Set = { - [s:string]: boolean + [s: string]: boolean }; class TypeScriptToDartTranspiler extends Writer { diff --git a/tools/broccoli/broccoli-writer.d.ts b/tools/broccoli/broccoli-writer.d.ts index 1505e4745d..ee52f3437f 100644 --- a/tools/broccoli/broccoli-writer.d.ts +++ b/tools/broccoli/broccoli-writer.d.ts @@ -1,8 +1,6 @@ /// -declare class Writer { - write(readTree: (tree) => Promise, destDir: string): Promise; -} +declare class Writer { write(readTree: (tree) => Promise, destDir: string): Promise; } export = Writer; diff --git a/tools/broccoli/traceur/index.ts b/tools/broccoli/traceur/index.ts index 5992fb21ed..1b2764054c 100644 --- a/tools/broccoli/traceur/index.ts +++ b/tools/broccoli/traceur/index.ts @@ -37,7 +37,7 @@ class TraceurFilter extends Writer { // TODO: we should fix the sourceMappingURL written by Traceur instead of overriding // (but we might switch to typescript first) var mapFilepath = filepath.replace(/\.\w+$/, '') + this.destSourceMapExtension; - result.js = result.js + `\n//# sourceMappingURL=./${path.basename(mapFilepath)}`; + result.js = result.js + '\n //# sourceMappingURL=./' + path.basename(mapFilepath); var destFilepath = filepath.replace(/\.\w+$/, this.destExtension); var destFile = path.join(destDir, destFilepath); diff --git a/tools/broccoli/ts2dart.d.ts b/tools/broccoli/ts2dart.d.ts index 5b93f4cc60..3dfc6c2778 100644 --- a/tools/broccoli/ts2dart.d.ts +++ b/tools/broccoli/ts2dart.d.ts @@ -8,7 +8,7 @@ export interface TranspilerOptions { basePath?: string; } -export class Transpiler{ +export class Transpiler { constructor(options: TranspilerOptions); transpile(fileNames: string[], outdir?: string); } diff --git a/tools/broccoli/typescript/index.ts b/tools/broccoli/typescript/index.ts index bb01e178ee..fb2579f09f 100644 --- a/tools/broccoli/typescript/index.ts +++ b/tools/broccoli/typescript/index.ts @@ -22,34 +22,34 @@ class TSCompiler extends Writer { } options.target = (ts).ScriptTarget[options.target]; return readTree(this.inputTree) - .then(srcDir => { - var files = walkSync(srcDir) - .filter(filepath => path.extname(filepath).toLowerCase() === '.ts') - .map(filepath => path.resolve(srcDir, filepath)); + .then(srcDir => { + var files = walkSync(srcDir) + .filter(filepath => path.extname(filepath).toLowerCase() === '.ts') + .map(filepath => path.resolve(srcDir, filepath)); - if (files.length > 0) { - var program = ts.createProgram(files, options); - var emitResult = program.emit(); + if (files.length > 0) { + var program = ts.createProgram(files, options); + var emitResult = program.emit(); - var allDiagnostics = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics); + var allDiagnostics = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics); - var errMsg = ''; - allDiagnostics.forEach(diagnostic => { - var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'); - if (!diagnostic.file) { - errMsg += `\n${message}`; - return; + var errMsg = ''; + allDiagnostics.forEach(diagnostic => { + var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'); + if (!diagnostic.file) { + errMsg += `\n${message}`; + return; + } + var {line, character} = + diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); + errMsg += `\n${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message}`; + }); + + if (emitResult.emitSkipped) { + throw new Error(errMsg); } - var {line, character} = - diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); - errMsg += `\n${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message}`; - }); - - if (emitResult.emitSkipped) { - throw new Error(errMsg); } - } - }); + }); } } module.exports = TSCompiler;