Skip to content

refactor: break up context.js and clean it up a bit#3794

Merged
WilcoFiers merged 2 commits intodevelopfrom
context-refactor
Nov 23, 2022
Merged

refactor: break up context.js and clean it up a bit#3794
WilcoFiers merged 2 commits intodevelopfrom
context-refactor

Conversation

@WilcoFiers
Copy link
Copy Markdown
Contributor

@WilcoFiers WilcoFiers commented Nov 22, 2022

In refactoring the context file I found this bug:

Related to #3793

@WilcoFiers WilcoFiers requested a review from a team as a code owner November 22, 2022 13:00
@WilcoFiers WilcoFiers marked this pull request as draft November 22, 2022 13:07
@WilcoFiers WilcoFiers changed the title fix(context): Allow multiple strings for include/exclude refactor: break up context.js and clean it up a bit Nov 23, 2022
Comment on lines -185 to -191
context.frames.forEach((frame, i) => {
if (frame.include.length === 0) {
return new Error(
'No elements found for include in Context of frame ' + i
);
}
});
Copy link
Copy Markdown
Contributor Author

@WilcoFiers WilcoFiers Nov 23, 2022

Choose a reason for hiding this comment

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

I left this out because it never worked. We're returning in a forEach. That does nothing. This is completely pointless too. To test the entire frame, include has to be empty. An empty include is a valid option.

@WilcoFiers WilcoFiers marked this pull request as ready for review November 23, 2022 13:58
@WilcoFiers WilcoFiers merged commit 757a6fc into develop Nov 23, 2022
@WilcoFiers WilcoFiers deleted the context-refactor branch November 23, 2022 21:06
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.

2 participants