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

Update v7.0 #155

Merged
merged 42 commits into from Dec 31, 2016
Merged

Update v7.0 #155

merged 42 commits into from Dec 31, 2016

Conversation

t32k
Copy link
Owner

@t32k t32k commented Oct 28, 2016

  • Convert Callback APIs to Promises
  • No longer support Node.js v4.0, use v6.0
  • Rewrite some code in ES6
  • Update module dependencies
  • Use ava test runner instead of mocha
  • Drop test specs feature
  • Drop Handlebars(no longer support custom,/markdown/html format)
  • Add prettify flag and function Prettify data #106

@t32k t32k added this to the v7.0.0 milestone Oct 28, 2016
@t32k t32k self-assigned this Oct 28, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 97.855% when pulling 2a0e7b2 on v7.0 into 6cdf422 on master.

@t32k t32k requested a review from 1000ch December 28, 2016 20:28
Copy link
Collaborator

@1000ch 1000ch left a comment

Choose a reason for hiding this comment

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

Where is PostCSS?

"template-curly-spacing": "error",
"yield-star-spacing": "error"
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use xo instead of eslint and its config.

console.log(JSON.stringify(result, null, 2));
});
stats.parse()
.then(function(result) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why function?

const StyleStats = require('../lib/stylestats');
const Format = require('../lib/format');
const specs = require('../lib/specs');
const util = require('../lib/util');
Copy link
Collaborator

Choose a reason for hiding this comment

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

util and fotmat should be exposed as property of stylestats.
This requires refactoring.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Next time! I believe you can do that :)

const path = require('path');
const json2csv = require('json2csv');
const Table = require('cli-table');
const Handlebars = require('handlebars');
Copy link
Collaborator

Choose a reason for hiding this comment

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

How custom template feature used? Can be dropped?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Next time, too.

callback(JSON.stringify(this.data, null, 2));
};
toJSON() {
return JSON.stringify(this.context, null, 2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why named context?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Followed sample codes on the official site.

// they will be joined into css string
this.cssFiles.forEach((cssFile) => {
// push local css data
that.styles.push(fs.readFileSync(cssFile, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should use fs.readFile().

* @returns {Promise}
*/
parse() {
const that = this;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why use that?

statsResult = result;
done();
}).catch(function(error){
throw error;
Copy link
Collaborator

Choose a reason for hiding this comment

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

!?

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 96.781% when pulling 1c378f0 on v7.0 into 6cdf422 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 96.781% when pulling f0e8d8c on v7.0 into 6cdf422 on master.

@t32k t32k requested review from 1000ch and removed request for 1000ch December 31, 2016 09:19
# The first commit's message is:
Fix configs

# This is the 2nd commit message:

Update travis.yml

# This is the 3rd commit message:

Update travis.yml

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date:      Fri Dec 30 03:39:38 2016 +0900
#
# interactive rebase in progress; onto 0eb5246
# Last commands done (5 commands done):
#    s f0e8d8c Update travis.yml
#    s 26b63d2 Update travis.yml
# Next commands to do (5 remaining commands):
#    pick 62f7344 Drop handlebars
#    pick 25c59b4 Add prettify flag
# You are currently editing a commit while rebasing branch 'v7.0' on '0eb5246'.
#
# Changes to be committed:
#	modified:   .travis.yml
#	modified:   package.json
#
# The first commit's message is:

Add prettify flag

# This is the 2nd commit message:

Update Readme

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date:      Fri Dec 30 04:42:23 2016 +0900
#
# interactive rebase in progress; onto 0eb5246
# Last commands done (8 commands done):
#    pick 25c59b4 Add prettify flag
#    s 1bd4496 Update Readme
# Next commands to do (2 remaining commands):
#    pick bed2b91 Add prettify method
#    pick 6a98da4 Use this keyword
# You are currently editing a commit while rebasing branch 'v7.0' on '0eb5246'.
#
# Changes to be committed:
#	modified:   README.md
#	modified:   bin/cli.js
#	modified:   lib/format.js
#	modified:   lib/stylestats.js
#
@t32k t32k merged commit 16b2839 into master Dec 31, 2016
@t32k t32k deleted the v7.0 branch December 31, 2016 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants