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

[#1047] v_summary: simplify toDisplay user logic #1051

Merged
merged 3 commits into from
Mar 5, 2020

Conversation

openorclose
Copy link

@openorclose openorclose commented Feb 4, 2020

Fixes #1047

The `toDisplay` logic does:
1. splits the search by space into terms
2. filters out any empty terms
3. maps each term to a bool,
  true if the term exists in `user.searchPath`
4. reduces the array to a single boolean,
  true if at least one true is in the array

The third and fourth steps can be combined,
to better reflect what is trying to be done.

Let's simplify by doing:
1. splits the search by space into terms (unchanged)
2. filters out any empty terms 
  (by using the more explicit `Boolean` function)
3. checks if at least one term exists in `user.searchPath`
  (by using `Array.prototype.some`)

Copy link
Contributor

@jamessspanggg jamessspanggg 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 for contributing!

@jamessspanggg jamessspanggg requested review from a team February 4, 2020 03:28
Copy link
Contributor

@Tejas2805 Tejas2805 left a comment

Choose a reason for hiding this comment

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

LGTM! Can you just improve your commit message?

@openorclose
Copy link
Author

LGTM! Can you just improve your commit message?

Hi, thanks for the review. I followed the message guidelines recommended here:
https://oss-generic.github.io/process/docs/FormatsAndConventions.html#commit-message

Any other suggestions to improve the commit message?

@Tejas2805
Copy link
Contributor

Any other suggestions to improve the commit message?

All looks good.

@fzdy1914 your review please.

@Tejas2805 Tejas2805 requested a review from a team February 19, 2020 15:15
@eugenepeh eugenepeh removed the request for review from a team February 23, 2020 05:14
@jamessspanggg jamessspanggg requested a review from a team March 1, 2020 01:39
@jinyao-lee jinyao-lee requested a review from fzdy1914 March 1, 2020 02:34
@fzdy1914 fzdy1914 changed the title [#1047] Simplify toDisplay user logic in v_summary [#1047] v_summary: simplify toDisplay user logic Mar 5, 2020
@fzdy1914 fzdy1914 merged commit 4d38d29 into reposense:master Mar 5, 2020
Tejas2805 added a commit to Tejas2805/RepoSense that referenced this pull request Mar 6, 2020
* 'master' of https://github.com/reposense/RepoSense:
  [reposense#1047] v_summary: simplify toDisplay user logic (reposense#1051)
  [reposense#658] Modify checkstyle configuration (reposense#1094)
  Add commit message length configuration border (reposense#1048)
  [reposense#1061] build.gradle: remove unused dependency (reposense#1095)
  [reposense#1044] Update date hashes on reset date range (reposense#1068)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hard to understand code snippet in v_summary.js
5 participants