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

Add initial value to reduce function for tab widths #3377

Merged

Conversation

fastjames
Copy link
Contributor

@fastjames fastjames commented Oct 10, 2019

Description
When calculating total tab width, we recently saw a case where there
were no tabs and thus no IV for reduce. This adds a default starting
value of 0 which prevents a js exception in that case. I checked the rest of the js and found no other occurrences of a reduce call without an initial value.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)

@fastjames
Copy link
Contributor Author

Edited desc to be clear: I have not added tests to this for two reasons:

  1. The addition of a default initial value of 0 for a summation function seems pretty obvious
  2. I didn't see much if any existing test code for javascript in this area, so I would have to start from scratch

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@spaghetticode
Copy link
Member

@fastjames can you please rebase with master, so we get a green CI? thank you 🙏

When calculating total tab width, we recently saw a case where there
were no tabs and thus no IV for reduce. This adds a default starting
value of 0 which prevents a js exception.
@fastjames fastjames force-pushed the default_initial_value_for_tab_widths branch from 60de8a9 to 346d75c Compare October 21, 2019 14:18
@fastjames
Copy link
Contributor Author

Rebased and force-pushed.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks!

@kennyadsl kennyadsl merged commit 7363581 into solidusio:master Oct 23, 2019
@kennyadsl kennyadsl deleted the default_initial_value_for_tab_widths branch October 23, 2019 17:11
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.

4 participants