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

feat: add filter, map, reduce options #66

Merged
merged 10 commits into from
Aug 9, 2017
Merged

Conversation

mastilver
Copy link
Contributor

@mastilver mastilver commented Jul 28, 2017

closes #46
closes #55
closes #35

@codecov-io
Copy link

codecov-io commented Jul 28, 2017

Codecov Report

Merging #66 into master will decrease coverage by 1.4%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #66      +/-   ##
==========================================
- Coverage     100%   98.59%   -1.41%     
==========================================
  Files           2        2              
  Lines          56       71      +15     
==========================================
+ Hits           56       70      +14     
- Misses          0        1       +1
Impacted Files Coverage Δ
lib/plugin.js 98.57% <97.36%> (-1.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c441207...7aa58c4. Read the comment docs.

@mastilver mastilver force-pushed the add-filter-options branch 2 times, most recently from 9cf27b2 to 0b413f8 Compare July 28, 2017 17:04
}, {
manifestOptions: {
filter: function(entry) {
return entry.isInitial;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@a-x- Does that looks good to you?

@mastilver mastilver added this to the v1.3 milestone Jul 29, 2017
@mastilver mastilver changed the title feat: add filter options feat: add filter, map, reduce options Jul 29, 2017
Copy link
Contributor

@a-x- a-x- left a comment

Choose a reason for hiding this comment

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

Minor changes will be good.
Looks good to me at all.

README.md Outdated

Type: `function`

Filter out files.
Copy link
Contributor

@a-x- a-x- Jul 31, 2017

Choose a reason for hiding this comment

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

I think will be cool to add examples here and below:
e.g.

// #48 Include initial chunks only (w/o dynamic dependencies)
filter: ({chunk, file}) => {
  return chunk.isInitial();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, I don't really want to pollute the readme too much...

manifestOptions: {
reduce: function (manifest, file) {
manifest[file.name] = {
file:file.path,
Copy link
Contributor

Choose a reason for hiding this comment

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

:<space>

Copy link
Contributor

@a-x- a-x- Jul 31, 2017

Choose a reason for hiding this comment

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

are we need eslint in CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We definitely need to setup eslint as some point

I personally have a preference for https://github.com/sindresorhus/xo , but really I don't mind ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

is xo like prettier?

README.md Outdated

Type: `function`

Create the manifest. It must return an `Object`. The default one will take the name of the chunk as a key and the path of the file as the value.
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand, we should note here: «custom reduce opt function replaces that default reduce logic:

function (manifest, file) {
        manifest[file.name] = file.path;
        return manifest;
}

@mastilver mastilver mentioned this pull request Aug 1, 2017
lib/plugin.js Outdated
manifest = files.reduce(function (manifest, file) {
manifest[file.name] = file.path;
return manifest;
}, {});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use seed instead of {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants