X Tutup
Skip to content

Commit daf0f47

Browse files
committed
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.
1 parent a3decad commit daf0f47

File tree

8 files changed

+46
-34
lines changed

8 files changed

+46
-34
lines changed

Brocfile-dart.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ var path = require('path');
66

77
// Transpile everything in 'modules'...
88
var modulesTree = new Funnel('modules', {
9-
include: ['**/*.js', '**/*.ts', '**/*.dart'], // .dart file available means don't translate.
9+
include: ['**/*.js', '**/*.ts', '**/*.dart'], // .dart file available means don't translate.
1010
exclude: ['rtts_assert/**/*'], // ... except for the rtts_asserts (don't apply to Dart).
11-
destDir: '/', // Remove the 'modules' prefix.
11+
destDir: '/', // Remove the 'modules' prefix.
1212
});
1313

1414
// Transpile to dart.

gulpfile.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,9 +504,22 @@ gulp.task('build/format.dart', rundartpackage(gulp, gulpPlugins, {
504504
args: CONFIG.formatDart.args
505505
}));
506506

507+
function doCheckFormat() {
508+
return gulp.src(['Brocfile*.js', 'modules/**/*.ts', 'tools/**/*.ts', '!**/typings/**/*.d.ts'])
509+
.pipe(format.checkFormat('file'));
510+
}
511+
507512
gulp.task('check-format', function() {
508-
return gulp.src(['Brocfile*.js', 'modules/**/*.ts', '!**/typings/**/*.d.ts'])
509-
.pipe(format.checkFormat('file'));
513+
return doCheckFormat().on('warning', function(e) {
514+
console.log("NOTE: this will be promoted to an ERROR in the continuous build");
515+
});
516+
});
517+
518+
gulp.task('enforce-format', function() {
519+
return doCheckFormat().on('warning', function(e) {
520+
console.log("ERROR: Some files need formatting");
521+
process.exit(1);
522+
});
510523
});
511524

512525
// ------------
@@ -752,6 +765,7 @@ gulp.task('build.js.dev', function(done) {
752765
runSequence(
753766
'broccoli.js.dev',
754767
'build/checkCircularDependencies',
768+
'check-format',
755769
done
756770
);
757771
});

scripts/ci/build_js.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ SCRIPT_DIR=$(dirname $0)
88
source $SCRIPT_DIR/env_dart.sh
99
cd $SCRIPT_DIR/../..
1010

11-
./node_modules/.bin/gulp build.js docs
11+
./node_modules/.bin/gulp enforce-format build.js docs

tools/broccoli/broccoli-ts2dart.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import path = require('path');
99
import ts2dart = require('ts2dart');
1010

1111
type Set = {
12-
[s:string]: boolean
12+
[s: string]: boolean
1313
};
1414

1515
class TypeScriptToDartTranspiler extends Writer {
Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/// <reference path="../typings/es6-promise/es6-promise.d.ts" />
22

33

4-
declare class Writer {
5-
write(readTree: (tree) => Promise<string>, destDir: string): Promise<any>;
6-
}
4+
declare class Writer { write(readTree: (tree) => Promise<string>, destDir: string): Promise<any>; }
75

86
export = Writer;

tools/broccoli/traceur/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class TraceurFilter extends Writer {
3737
// TODO: we should fix the sourceMappingURL written by Traceur instead of overriding
3838
// (but we might switch to typescript first)
3939
var mapFilepath = filepath.replace(/\.\w+$/, '') + this.destSourceMapExtension;
40-
result.js = result.js + `\n//# sourceMappingURL=./${path.basename(mapFilepath)}`;
40+
result.js = result.js + '\n //# sourceMappingURL=./' + path.basename(mapFilepath);
4141

4242
var destFilepath = filepath.replace(/\.\w+$/, this.destExtension);
4343
var destFile = path.join(destDir, destFilepath);

tools/broccoli/ts2dart.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export interface TranspilerOptions {
88
basePath?: string;
99
}
1010

11-
export class Transpiler{
11+
export class Transpiler {
1212
constructor(options: TranspilerOptions);
1313
transpile(fileNames: string[], outdir?: string);
1414
}

tools/broccoli/typescript/index.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,34 +22,34 @@ class TSCompiler extends Writer {
2222
}
2323
options.target = (<any>ts).ScriptTarget[options.target];
2424
return readTree(this.inputTree)
25-
.then(srcDir => {
26-
var files = walkSync(srcDir)
27-
.filter(filepath => path.extname(filepath).toLowerCase() === '.ts')
28-
.map(filepath => path.resolve(srcDir, filepath));
25+
.then(srcDir => {
26+
var files = walkSync(srcDir)
27+
.filter(filepath => path.extname(filepath).toLowerCase() === '.ts')
28+
.map(filepath => path.resolve(srcDir, filepath));
2929

30-
if (files.length > 0) {
31-
var program = ts.createProgram(files, options);
32-
var emitResult = program.emit();
30+
if (files.length > 0) {
31+
var program = ts.createProgram(files, options);
32+
var emitResult = program.emit();
3333

34-
var allDiagnostics = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics);
34+
var allDiagnostics = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics);
3535

36-
var errMsg = '';
37-
allDiagnostics.forEach(diagnostic => {
38-
var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n');
39-
if (!diagnostic.file) {
40-
errMsg += `\n${message}`;
41-
return;
42-
}
43-
var {line, character} =
44-
diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
45-
errMsg += `\n${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message}`;
46-
});
36+
var errMsg = '';
37+
allDiagnostics.forEach(diagnostic => {
38+
var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n');
39+
if (!diagnostic.file) {
40+
errMsg += `\n${message}`;
41+
return;
42+
}
43+
var {line, character} =
44+
diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
45+
errMsg += `\n${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message}`;
46+
});
4747

48-
if (emitResult.emitSkipped) {
49-
throw new Error(errMsg);
48+
if (emitResult.emitSkipped) {
49+
throw new Error(errMsg);
50+
}
5051
}
51-
}
52-
});
52+
});
5353
}
5454
}
5555
module.exports = TSCompiler;

0 commit comments

Comments
 (0)
X Tutup