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

fix(server): refactor file-list event interface #3616

Closed
wants to merge 5 commits into from

Conversation

johnjbarton
Copy link
Contributor

Remove the confusing promise container in favor of a current-file-list state.
The custom-then() function around a promise is hard to understand and reason about.
A simple object with a files property expresses the 'current files list' clearer.

@karmarunnerbot
Copy link
Member

Build karma 463 completed (commit 5c9bef693e by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2860 completed (commit 5c9bef693e by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 462 completed (commit 5c9bef693e by @johnjbarton)

lib/server.js Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

Build karma 2862 failed (commit f6d72807f0 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 465 failed (commit f6d72807f0 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 464 failed (commit f6d72807f0 by @johnjbarton)

Copy link
Collaborator

@devoto13 devoto13 left a comment

Choose a reason for hiding this comment

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

LGTM once CI is happy

Remove the confusing promise container in favor of a current-file-list state.
The custom-then() function around a promise is hard to understand and reason about.
A simple object with a files property expresses the 'current files list' clearer.
The input config supplies 'patterns' which FileList expands into Files.
These files are preprocessed and then 'file-list-modified' is fired.
The event offers a pair of file lists, {included, served}.
The bundle layer maps this pair to a new pair where fewer files are included.
By default the bundler copies the inputs to outputs.

Fixes karma-runner#3633
@johnjbarton
Copy link
Contributor Author

I added the bundler here. I will try to separate the addition from the refactoring.

@AppVeyorBot
Copy link

Build karma 2894 completed (commit f8614d3a6e by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 497 completed (commit f8614d3a6e by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 496 completed (commit f8614d3a6e by @johnjbarton)

@johnjbarton
Copy link
Contributor Author

To avoid introducing a change to middleware/karma and then just turn around and change it again, I combined the refactoring with the bundler introduction in #3638

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