Skip to content

cacheGroups #2227

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

Jump to bottom
Merged
merged 5 commits into from
Dec 16, 2018
Merged

cacheGroups #2227

merged 5 commits into from
Dec 16, 2018

Conversation

sharh
Copy link
Contributor

@sharh sharh commented Jun 6, 2018

addCache test correctly

describe your changes...

  • Read and sign the CLA. PRs that haven't signed it won't be accepted.
  • Make sure your PR complies with the writer's guide.
  • Review the diff carefully as sometimes this can reveal issues.
  • Remove these instructions from your PR as they are for your eyes only.

@sharh
cacheGroups
df83753 
addCache test correctly
@jsf-clabot
Copy link

jsf-clabot commented Jun 6, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

dhruvdutt
dhruvdutt reviewed Jun 6, 2018
View reviewed changes
Copy link
Member

@dhruvdutt dhruvdutt left a comment

Choose a reason for hiding this comment

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

Please sign CLA.

Legends
Legends reviewed Jun 13, 2018
View reviewed changes
Copy link
Member

@Legends Legends left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The title #### splitChunks.cacheGroups.test

should also be changed, one would think, the test property is on cacheGroups level, something like:

splitChunks.cacheGroups.cacheGroup.test should be correct.

EugeneHlushko
EugeneHlushko reviewed Jun 18, 2018
View reviewed changes
src/content/plugins/split-chunks-plugin.md Outdated
//...
return true
}
// cacheGroups test
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary comment

EugeneHlushko
EugeneHlushko approved these changes Jun 18, 2018
View reviewed changes
Copy link
Member

@EugeneHlushko EugeneHlushko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would suggest to remove the comment, otherwise a good spot!

If you want to extend the PR please also take care of the title, my suggestion would be:

splitChunks.cacheGroups.{cacheGroup}.test

@montogeek
Copy link
Member

Thanks!

@montogeek montogeek merged commit 5b3906e into webpack:master Dec 16, 2018
EugeneHlushko added a commit that referenced this pull request Dec 17, 2018
@EugeneHlushko
Revert "docs(plugins): Add title to cacheGroups (#2227)"
955d4a2 
This reverts commit 5b3906e.
@EugeneHlushko EugeneHlushko mentioned this pull request Dec 17, 2018
Merged
montogeek pushed a commit that referenced this pull request Dec 18, 2018
@EugeneHlushko @montogeek
Revert "cacheGroups" (#2725)
51232cc 
* Revert "docs(plugins): Add title to cacheGroups (#2227)"

This reverts commit 5b3906e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Legends Legends Legends left review comments

@dhruvdutt dhruvdutt dhruvdutt left review comments

@EugeneHlushko EugeneHlushko EugeneHlushko approved these changes

Assignees
No one assigned
Labels
Plugins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sharh @jsf-clabot @montogeek @Legends @dhruvdutt @EugeneHlushko