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

Do not crash when source plugins conflict. #3180

Closed
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
2 participants
@musically-ut
Contributor

musically-ut commented Nov 25, 2014

Fixes #3179.

This is one approach to solve the problem by keeping the pkg information around until we have finished loading all the source handlers. This information is discarded afterwards.

sourcePath: compileStep.inputPath
};
allHandlersWithPkgs['js'] = {
pkg: { /* native handler */ },

This comment has been minimized.

@glasser

glasser Dec 5, 2014

Member

Let's just store packageName (or null) directly instead of pkg that we only use for its name

sourceExtensions[ext] = !!sourceHandler.isTemplate;
});
});
var allHandlers = {};

This comment has been minimized.

@glasser

glasser Dec 5, 2014

Member

Let's just use allHandlersWithPkgs later in the function instead of having two similar data structures

@glasser

This comment has been minimized.

Member

glasser commented Dec 5, 2014

Thanks! Comments inline.

Incorporate merge comments.
 1. Use allHandlersWithPkgs instead of duplicating data in allHandlers
 2. Save only the pkg names instead of the whole packages
@musically-ut

This comment has been minimized.

Contributor

musically-ut commented Dec 5, 2014

@glasser Have incorporated the feedback into the PR. I can rebase off current devel and create a new pull request if there are conflicts.

glasser added a commit that referenced this pull request Dec 11, 2014

@glasser

This comment has been minimized.

Member

glasser commented Dec 11, 2014

Thanks, merged!

@glasser glasser closed this Dec 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment