Skip to content
This repository has been archived by the owner on Feb 17, 2022. It is now read-only.

Fixed format-versions when problem bundle does not contain the versions #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maoueh
Copy link

@maoueh maoueh commented Jan 17, 2018

It appears that it's possible the problem bundle received from webpack does
not contain the versions data at all. This was not handled correctly in
the formatVersions function.

Fixed the function to be more robust and added unit tests for it along the
way. Unit tests for an existing case was added but really not sure if it's
the right format as I never seen it :)

Note I disabled no-use-before-define in my unit test as I found it
makes tests much more readable. Don't hesitate to disapprove and I'll
revert the change.

It appears that it's possible the problem bundle received from webpack does
not contain the `versions` data at all. This was not handled correctly in
the `formatVersions` function.

Fixed the function to be more robust and added unit tests for it along the
way. Unit tests for an existing case was added but really not sure if it's
the right format as I never seen it :)

**Note** I disabled `no-use-before-define` in my unit test as I found it
makes tests much more readable. Don't hesitate to disapprove and I'll
revert the change.
});
});

const Problem = {
Copy link
Contributor

@mattcubitt mattcubitt Feb 1, 2018

Choose a reason for hiding this comment

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

Hi @maoueh I would prefer you re-enabled the rule and moved the declarations to the top. I actually find this more readable and consistent with the rest of the code base. The fix looks good to me so will merge when you've done this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants