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.

Sign up for GitHub

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

Do not build graph by default #357

Merged
merged 3 commits into from
Sep 23, 2015
Merged

Do not build graph by default #357

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d052842
make: create build graph only when it needed
j0tunn Sep 11, 2015
5f97701
Separate class NodeWithGraph for building graph
j0tunn Sep 11, 2015
bbb78f3
codestyle
j0tunn Sep 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
codestyle
  • Loading branch information
@j0tunn
j0tunn committed Sep 23, 2015
commit bbb78f33001b89771742e34d1a692cf8719b3bc1
15 changes: 14 additions & 1 deletion .jscsrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,20 @@
"disallowQuotedKeysInObjects": "allButReserved",
"disallowSpaceAfterObjectKeys": true,
"requireCommaBeforeLineBreak": true,
"requireOperatorBeforeLineBreak": true,
"requireOperatorBeforeLineBreak": [
"=",
"-",
"/",
"*",
"==",
"===",
"!=",
"!==",
">",
">=",
"<",
"<="
],
"disallowSpaceAfterPrefixUnaryOperators": true,
"disallowSpaceBeforePostfixUnaryOperators": true,
"requireSpaceBeforeBinaryOperators": true,
Expand Down
2 changes: 1 addition & 1 deletion lib/api/make.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = function (targets, options) {
logger,
graph;

return makePlatform.init(root, options.mode, options.config, {graph: options.graph})
return makePlatform.init(root, options.mode, options.config, { graph: options.graph })
.then(function () {
logger = makePlatform.getLogger();

Expand Down
12 changes: 3 additions & 9 deletions lib/node/node-with-graph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@ module.exports = inherit(Node, {
this._graph = graph;
},

///
_initTech: function (tech) {
var _this = this;
var nodeAdapterClass = function () {};
nodeAdapterClass.prototype = _this;
var nodeAdapter = new nodeAdapterClass();
var NodeAdapter = function () {};
NodeAdapter.prototype = _this;
var nodeAdapter = new NodeAdapter();

return Vow.when(tech.init(nodeAdapter)).then(function () {
return Vow.when(tech.getTargets()).then(function (targets) {
Expand All @@ -46,7 +45,6 @@ module.exports = inherit(Node, {
});
},

///
requireSources: function (sources, targets) {
Copy link
Member

Choose a reason for hiding this comment

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

Сейчас методы переопределяются сразу, а не от наличия this._graph как было раньше.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

В этом и смысл: это отдельный класс, в котором this._graph точно есть. А какую ноду создавать (с графом или без) решается в фабричном методе по наличию аргумента graph

Copy link
Member

Choose a reason for hiding this comment

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

Тогда не нужен агрумент graph в конструкторе.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

как это не нужен? Он же из makePlatform летит, и в конструкторе сохраняется в свойство this._graph

var _this = this;
this._addDepsToGraph(targets, sources, function (source) {
Expand All @@ -56,7 +54,6 @@ module.exports = inherit(Node, {
return Node.prototype.requireSources.apply(this, arguments);
},

///
requireNodeSources: function (sources, targets) {
var _this = this;
_.forEach(sources, function (nodeSources, nodeName) {
Expand All @@ -68,7 +65,6 @@ module.exports = inherit(Node, {
return Node.prototype.requireNodeSources.apply(this, arguments);
},

///
_addDepsToGraph: function (targets, sources, getDepFromPath) {
var _this = this;
targets = targets || [];
Expand All @@ -84,13 +80,11 @@ module.exports = inherit(Node, {
});
},

///
resolveTarget: function (target) {
this._graph.resolveTarget(path.join(this._path, target));
return Node.prototype.resolveTarget.apply(this, arguments);
},

///
destruct: function () {
this.__base();
delete this._graph;
Expand Down
2 changes: 1 addition & 1 deletion test/lib/make/init-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('make/initNode', function () {
vowFs.makeDir.returns(vow.fulfill()); // prevent temp dir creation on MakePlatform.init()

makePlatform = new MakePlatform();
makePlatform.init('/path/to/project', 'mode', function () {}, {graph: true});
makePlatform.init('/path/to/project', 'mode', function () {}, { graph: true });
});

afterEach(function () {
Expand Down
4 changes: 2 additions & 2 deletions test/lib/make/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ describe('make/init', function () {
});

it('should create build graph on demand', function () {
init_({opts: {graph: true}});
init_({ opts: { graph: true } });

expect(makePlatform.getBuildGraph()).to.be.instanceOf(BuildGraph);
});

it('should initialize build graph with project name', function () {
init_({ projectPath: '/path/to/project-name', opts: {graph: true}});
init_({ projectPath: '/path/to/project-name', opts: { graph: true } });

expect(makePlatform.getBuildGraph().__constructor).to.be.calledWith('project-name');
});
Expand Down
4 changes: 2 additions & 2 deletions test/lib/node/node-with-graph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ describe('node-with-graph', function () {
sandbox.stub(Node.prototype, 'requireNodeSources');
Node.prototype.requireNodeSources.returns(vow.resolve());

return node.requireNodeSources({'node': ['?.js']}, ['target.js'])
return node.requireNodeSources({ node: ['?.js'] }, ['target.js'])
.then(function () {
expect(graph.addDep).to.be.calledWith(
path.join(nodePath, 'target.js'),
path.join('node', 'node.js')
);
expect(Node.prototype.requireNodeSources).to.be.calledWith({'node': ['?.js']}, ['target.js']);
expect(Node.prototype.requireNodeSources).to.be.calledWith({ node: ['?.js'] }, ['target.js']);
});
});
});
Expand Down