-
Notifications
You must be signed in to change notification settings - Fork 11
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
[GPT-747][MAJOR] Unified compiler #15
Conversation
@@ -0,0 +1,11 @@ | |||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to investigate https://github.com/puleos/object-hash
It is widely used, dep free, well maintained, apparently, and defaults to sha1+hex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can take care of it inside OC as separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! We can definitely use that instead. Thanks for pointing that out 💅
addresses #17 ✅ |
addresses #18 |
addresses #20 |
Test Suites: 16 passed, 16 total
Tests: 43 passed, 43 total
Snapshots: 30 passed, 30 total
Time: 6.374s
Ran all test suites.
-----------------------------------------|----------|----------|----------|----------|----------------|
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
-----------------------------------------|----------|----------|----------|----------|----------------|
All files | 92.34 | 72.22 | 97.5 | 92.27 | |
infinite-loop-loader | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
infinite-loop-loader/lib | 96.55 | 82.61 | 100 | 96.55 | |
infinite-loop-loader.js | 90 | 63.64 | 100 | 90 | 9 |
transformLoopWithLimit.js | 100 | 100 | 100 | 100 | |
oc-external-dependencies-handler | 94.12 | 70 | 100 | 94.12 | |
index.js | 94.12 | 70 | 100 | 94.12 | 35 |
oc-get-unix-utc-timestamp | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-hash-builder | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-minify-file | 83.33 | 25 | 100 | 83.33 | |
index.js | 83.33 | 25 | 100 | 83.33 | 24,25 |
oc-template-jade | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-template-jade-compiler | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-template-jade-compiler/lib | 94.74 | 75 | 100 | 94.69 | |
compile.js | 91.89 | 60 | 100 | 91.67 | 31,47,65 |
compileServer.js | 94.44 | 62.5 | 100 | 94.44 | 27 |
compileStatics.js | 100 | 94.44 | 100 | 100 | 14 |
compileView.js | 90.91 | 50 | 100 | 90.91 | 19,48 |
oc-template-jade-compiler/lib/resources | 100 | 100 | 100 | 100 | |
strings.js | 100 | 100 | 100 | 100 | |
oc-template-jade/lib | 76.92 | 100 | 66.67 | 75 | |
getCompiledTemplate.js | 50 | 100 | 0 | 50 | 5,6,7 |
getInfo.js | 100 | 100 | 100 | 100 | |
render.js | 100 | 100 | 100 | 100 | |
oc-view-wrapper | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-webpack | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
oc-webpack/lib/compiler | 76.19 | 58.33 | 100 | 76.19 | |
index.js | 76.19 | 58.33 | 100 | 76.19 | 19,25,26,30,36 |
oc-webpack/lib/configurator | 100 | 100 | 100 | 100 | |
index.js | 100 | 100 | 100 | 100 | |
-----------------------------------------|----------|----------|----------|----------|----------------|
@mattiaerre ^^ 💅 🌈 🕶 |
@matteofigus should be ready to be reviewd/merged. |
let's aim for 100% code coverage @nickbalestra 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of super little changes
.travis.yml
Outdated
- node_js: "4" | ||
- node_js: "5" | ||
# - node_js: "4" | ||
# - node_js: "5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just remove 4 and 5? Thanks
@@ -0,0 +1,60 @@ | |||
# infinite-loop-loader | |||
[![Build Status](https://travis-ci.org/nickbalestra/infinite-loop-loader.svg?branch=master)](https://travis-ci.org/nickbalestra/infinite-loop-loader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong travis url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! This could be removed, as we have now the travis badge on root for the whole monorepo
@@ -0,0 +1,60 @@ | |||
# infinite-loop-loader | |||
[![Build Status](https://travis-ci.org/nickbalestra/infinite-loop-loader.svg?branch=master)](https://travis-ci.org/nickbalestra/infinite-loop-loader) | |||
[![Standard - JavaScript Style Guide](https://img.shields.io/badge/code%20style-standard-brightgreen.svg)](http://standardjs.com/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove standard badge
@@ -0,0 +1 @@ | |||
# oc-external-dependencies-handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add just a little description of what this is. Same for all the other modules. At least one single sentence that matches package's "description" field.
@@ -0,0 +1 @@ | |||
# oc-get-unix-utc-timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, just a little description. Also, can we remove 'oc' from the name? I think this is more of a generic "get unix utc timestamp" module.
@@ -0,0 +1,5 @@ | |||
const getTimeStamp = require('../index.js'); | |||
|
|||
test('Generates correct timestamps everytime its invoked', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's
const compileStatics = require('./compileStatics'); | ||
const getUnixUtcTimestamp = require('oc-get-unix-utc-timestamp'); | ||
|
||
// OPTIONS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make this list alphabetic. Also, I don't see here "ocPackage"
@matteofigus done |
No description provided.