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

fix(formatHit): fixed npe, not all hits have a country #292

Merged
merged 1 commit into from
Jul 20, 2016
Merged

Conversation

redox
Copy link
Contributor

@redox redox commented Jul 20, 2016

Fix #291

@redox redox merged commit ff54319 into master Jul 20, 2016
@redox redox deleted the fixed-npe branch July 20, 2016 17:00
@@ -2,6 +2,9 @@ import findCountryCode from './findCountryCode.js';
import findType from './findType.js';

function getBestHighlightedForm(highlightedValues) {
if (highlightedValues.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the API sometimes send _highlightResult.locale_names = []?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we don't but I prefer having too much safety checks

Copy link
Contributor

@vvo vvo Jul 21, 2016

Choose a reason for hiding this comment

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

Ok, I would remove it then. I feel like everytime we do this, it leads to defensive programming, dead code + FUD (= we will keep this code, just in case)

@vvo
Copy link
Contributor

vvo commented Jul 21, 2016

Add tests pleaaaaase :p

vvo pushed a commit that referenced this pull request Jul 21, 2016
@redox
Copy link
Contributor Author

redox commented Jul 21, 2016

Add tests pleaaaaase :p

Yes sorry about that, has been done quickly to fix the broken latest version.

@vvo
Copy link
Contributor

vvo commented Jul 21, 2016

added one in #293

vvo pushed a commit that referenced this pull request Jul 21, 2016
vvo added a commit that referenced this pull request Jul 25, 2016
* test(formatHit): add test when no country

linked with #292

* chore(formatHit): remove dead code
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