Skip to content

Commit

Permalink
feat(build): enforce formatting of some files.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alexeagle committed Apr 12, 2015
1 parent a3decad commit daf0f47
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 34 deletions.
4 changes: 2 additions & 2 deletions Brocfile-dart.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 16 additions & 2 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

// ------------
Expand Down Expand Up @@ -752,6 +765,7 @@ gulp.task('build.js.dev', function(done) {
runSequence(
'broccoli.js.dev',
'build/checkCircularDependencies',
'check-format',
done
);
});
Expand Down
2 changes: 1 addition & 1 deletion scripts/ci/build_js.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tools/broccoli/broccoli-ts2dart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import path = require('path');
import ts2dart = require('ts2dart');

type Set = {
[s:string]: boolean
[s: string]: boolean
};

class TypeScriptToDartTranspiler extends Writer {
Expand Down
4 changes: 1 addition & 3 deletions tools/broccoli/broccoli-writer.d.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/// <reference path="../typings/es6-promise/es6-promise.d.ts" />


declare class Writer {
write(readTree: (tree) => Promise<string>, destDir: string): Promise<any>;
}
declare class Writer { write(readTree: (tree) => Promise<string>, destDir: string): Promise<any>; }

export = Writer;
2 changes: 1 addition & 1 deletion tools/broccoli/traceur/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tools/broccoli/ts2dart.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export interface TranspilerOptions {
basePath?: string;
}

export class Transpiler{
export class Transpiler {
constructor(options: TranspilerOptions);
transpile(fileNames: string[], outdir?: string);
}
46 changes: 23 additions & 23 deletions tools/broccoli/typescript/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,34 +22,34 @@ class TSCompiler extends Writer {
}
options.target = (<any>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 {line, character} =
diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
errMsg += `\n${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message}`;
});
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);
if (emitResult.emitSkipped) {
throw new Error(errMsg);
}
}
}
});
});
}
}
module.exports = TSCompiler;

0 comments on commit daf0f47

Please sign in to comment.