Skip to content

3900 guest search api#3908

Closed
michbarsinai wants to merge 4 commits intodevelopfrom
3900-guest-search-api
Closed

3900 guest search api#3908
michbarsinai wants to merge 4 commits intodevelopfrom
3900-guest-search-api

Conversation

@michbarsinai
Copy link
Copy Markdown
Member

@michbarsinai michbarsinai commented Jun 14, 2017

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!

Related Issues

Pull Request Checklist

  • Unit tests completed
  • Integration tests: None
  • Deployment requirements: None
  • Documentation completed
  • Merged latest from "develop" branch and resolved conflicts

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.002%) to 10.282% when pulling d6f0d40 on 3900-guest-search-api into e903302 on develop.

return GuestUser.get();
}
return settingsSvc.isTrueForKey(SettingsServiceBean.Key.SearchApiRequireToken, false) ?
findAuthenticatedUserOrDie() : findUserOrDie();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this terinary form cleaner than the equivalent if/else?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Depends on your style. I like it since it makes more explicit the fact that there's a single value being decided on based on a condition.
I normally try to reserve if {..} else {..} (which is a statement rather than an expression) to different paths in an algorithm.

So, literature-like style reasons, no hard data here.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.002%) to 10.278% when pulling 7d7b5e3 on 3900-guest-search-api into 76d9fd3 on develop.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.002%) to 10.278% when pulling 501f5b1 on 3900-guest-search-api into 9708f57 on develop.

@pdurbin
Copy link
Copy Markdown
Member

pdurbin commented Jun 21, 2017

Closing in favor of pull request #3940.

@pdurbin pdurbin closed this Jun 21, 2017
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