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

refactor gulp build code #589

Merged
merged 14 commits into from
Feb 3, 2017
Merged

refactor gulp build code #589

merged 14 commits into from
Feb 3, 2017

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented Feb 1, 2017

decided to give the gulp code a little polishing ✨

  • remove unused getTypescriptSources (replaced by globs)
  • remove blueprint.dest(): unnecessary complexity
  • remove blueprint.taskMapper(): use projects.map() to build array once and avoid duplicating strings
  • move karma config to separate file (🎩 @codebling)
  • delete unused (and incomplete) mocha task
  • add a missing license block

gulp/index.js Outdated
@@ -8,27 +8,10 @@ const plugins = require("gulp-load-plugins")();
const assign = require("lodash/assign");
Copy link
Contributor

Choose a reason for hiding this comment

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

unused import?

@blueprint-bot
Copy link

refactor usage of taskMapper

Preview: docs
Coverage: core | datetime

@blueprint-bot
Copy link

refactor taskMapper usage

Preview: docs
Coverage: core | datetime

refactor away from the taskNames map pattern.
@blueprint-bot
Copy link

better taskMapper is more useful

Preview: docs
Coverage: core | datetime

@giladgray
Copy link
Contributor Author

I have an even sweeter refactor ready to go on top of this that hopefully resolves a lot of the ambient pain with this gulp build by making things much more explicit. But I'd like to open that as a separate review, so let's please get this one reviewed ASAP cuz it's still got some good relevant cleanup (like removing all those obsolete blueprint.* functions).

@@ -111,5 +33,7 @@ module.exports = (gulp, plugins, blueprint) => {
});
});

gulp.task("karma", (done) => rs(...blueprint.taskMapper("karma", "karma-"), done));
// running in sequence so output is human-friendly
// (in parallel, all suites get interspersed and it's a mess)
Copy link
Contributor

Choose a reason for hiding this comment

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

for another PR: try running expensive parallel tasks in separate CircleCI containers

@@ -1,3 +1,6 @@
/*
* Copyright 2015 Palantir Technologies, Inc. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2016?

@@ -1,5 +1,5 @@
/*
* Copyright 2015 Palantir Technologies, Inc. All rights reserved.
* Copyright 2017 Palantir Technologies, Inc. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

2016?

Gilad Gray added 3 commits February 2, 2017 22:58
move blueprint to the beginning so it's like two key libraries, then a mess of auxiliary plugins all at once. also now it's alphabetized 👌
replaced by lerna in scripts/prepRelease.sh
@giladgray
Copy link
Contributor Author

pushed some last-minute tweaks that came up in my other feature work but make sense to get in here. review the last few commits individually for best results.

@blueprint-bot
Copy link

copyright years

Preview: docs
Coverage: core | datetime

@giladgray giladgray added the P0 label Feb 3, 2017
@giladgray giladgray merged commit 7f20eb9 into master Feb 3, 2017
@giladgray giladgray deleted the gg/refactor-gulp branch February 3, 2017 19:26
@giladgray giladgray mentioned this pull request Feb 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants