[#1420] Extract some code from summary.js #1421

gerhean · 2021-01-19T13:42:36Z

Some of [#1420], but more can certainly be done.

Currently summary.js is very long (900+ lines).

This makes maintaining code hard 
as it not always clear which code causes
changes in the components state.

Ideally, all code which do not modify
state should be moved out.

This further helps to decouple code,
as it forces the dependencies of the functions
to be written clearly as arguments,
allowing the function to act as a reliable
black box which is also easier to test.

Lets move some code out of summary.js.

gerhean · 2021-01-19T15:03:01Z

frontend/src/static/js/utils/repo_sorter.js

+function getTotalCommits(total, group) {
+  return total + group.checkedFileTypeContribution;
+}
+
+function getGroupCommitsVariance(total, group) {
+  return total + group.variance;
+}
+
+// function getGroupCommitsVariance(sortingOption) {
+//   return function (total, group) {
+//     if (sortingOption === 'totalCommits') {
+//       return total + group.checkedFileTypeContribution;
+//     }
+//     return total + group[sortingOption];
+//   };
+// }
+
+function sortingHelper(element, sortingOption) {
+  // return sortingOption === 'totalCommits' || sortingOption === 'variance'
+  //     ? element.reduce(getGroupCommitsVariance, 0)
+  //     : element[0][sortingOption];
+  if (sortingOption === 'totalCommits') {
+    return element.reduce(getTotalCommits, 0);
+  }
+  if (sortingOption === 'variance') {
+    return element.reduce(getGroupCommitsVariance, 0);
+  }
+  return element[0][sortingOption];
+}


The change I made to the code. Will remove comments once verified is ok.

Any reason for making this change?

Actually I realized that the commented code was not the original code sorry.

The reason why I changed the original code is because it was using this.sortingOption which is not available when extracted, and that returning an anonymous function is not the best idea (I think there was a warning).

Tejas2805

Left a few initial comments.

Tejas2805 · 2021-01-20T13:02:10Z

frontend/src/static/js/utils/repo_sorter.js

+function getTotalCommits(total, group) {
+  return total + group.checkedFileTypeContribution;
+}
+
+function getGroupCommitsVariance(total, group) {
+  return total + group.variance;
+}
+
+// function getGroupCommitsVariance(sortingOption) {
+//   return function (total, group) {
+//     if (sortingOption === 'totalCommits') {
+//       return total + group.checkedFileTypeContribution;
+//     }
+//     return total + group[sortingOption];
+//   };
+// }
+
+function sortingHelper(element, sortingOption) {
+  // return sortingOption === 'totalCommits' || sortingOption === 'variance'
+  //     ? element.reduce(getGroupCommitsVariance, 0)
+  //     : element[0][sortingOption];
+  if (sortingOption === 'totalCommits') {
+    return element.reduce(getTotalCommits, 0);
+  }
+  if (sortingOption === 'variance') {
+    return element.reduce(getGroupCommitsVariance, 0);
+  }
+  return element[0][sortingOption];
+}


Any reason for making this change?

frontend/src/static/js/utils/ramp_colour_generator.js

frontend/src/static/js/v_summary.js

fzdy1914 · 2021-02-07T12:36:52Z

@gerhean Pls answer the questions and proceed with PR.

gerhean · 2021-02-15T01:35:01Z

Would it be good to merge this first and extract more stuff in another PR?

0blivious · 2021-02-15T16:10:31Z

What are the other changes you plan to extract? Can you give some line number?

gerhean · 2021-01-20T14:00:37Z

frontend/src/static/js/utils/repo_sorter.js

+function getTotalCommits(total, group) {
+  return total + group.checkedFileTypeContribution;
+}
+
+function getGroupCommitsVariance(total, group) {
+  return total + group.variance;
+}
+
+// function getGroupCommitsVariance(total, group) {
+//   if (this.sortingOption === 'totalCommits') {
+//     return total + group.checkedFileTypeContribution;
+//   }
+//   return total + group[this.sortingOption];
+// }
+
+function sortingHelper(element, sortingOption) {
+  // return sortingOption === 'totalCommits' || sortingOption === 'variance'
+  //     ? element.reduce(getGroupCommitsVariance, 0)
+  //     : element[0][sortingOption];
+  if (sortingOption === 'totalCommits') {
+    return element.reduce(getTotalCommits, 0);
+  }
+  if (sortingOption === 'variance') {
+    return element.reduce(getGroupCommitsVariance, 0);
+  }
+  return element[0][sortingOption];
+}


Actually I realized that the previously commented code was not the original code sorry.

The reason why I changed the original code is because it was using this.sortingOption which is not available when extracted, and that returning an anonymous function is not the best idea (I think there was a warning).

frontend/src/static/js/v_summary.js

gerhean · 2021-02-19T01:49:21Z

What are the other changes you plan to extract? Can you give some line number?

At the moment I'm not really focusing on this so nothing yet.

Repositories with more than 19 different types of files will crash, which can be traced from line 448 of `v_summary.js`. Bug is caused by a function not being copied properly in #1421, leading to an undefined function call. Let's copy the function over properly.

gerhean added 2 commits January 19, 2021 21:40

initial commit

277eb10

pass cypress

e16f1c2

gerhean mentioned this pull request Jan 19, 2021

[#795] Projectify frontend part #1412

Merged

gerhean closed this Jan 19, 2021

gerhean reopened this Jan 19, 2021

reduce line length

d8ddb1d

gerhean commented Jan 19, 2021

View reviewed changes

correct name

7b7283d

gerhean requested a review from a team January 19, 2021 15:26

Tejas2805 reviewed Jan 20, 2021

View reviewed changes

revert code in comments

926c2f9

0blivious reviewed Jan 21, 2021

View reviewed changes

frontend/src/static/js/v_summary.js Show resolved Hide resolved

Tejas2805 and others added 3 commits January 25, 2021 15:25

Merge branch 'master' into abstract-color-generator-and-sort-repo

dc5eed3

Merge branch 'master' into pr/1421

5ce057f

Merge branch 'master' into abstract-color-generator-and-sort-repo

b01804d

Merge branch 'master' into abstract-color-generator-and-sort-repo

92b7f0a

gerhean requested review from Tejas2805 and 0blivious February 15, 2021 01:35

gerhean commented Feb 16, 2021

View reviewed changes

0blivious approved these changes Feb 24, 2021

View reviewed changes

Merge branch 'master' into abstract-color-generator-and-sort-repo

f734228

fzdy1914 approved these changes Feb 24, 2021

View reviewed changes

fzdy1914 added 2 commits February 24, 2021 15:35

Merge branch 'master' into abstract-color-generator-and-sort-repo

e76c369

Merge branch 'master' into abstract-color-generator-and-sort-repo

c300ae7

fzdy1914 merged commit 0f70ff3 into reposense:master Mar 4, 2021

gerhean mentioned this pull request Mar 7, 2021

Dashboard loading empty if base URL is given #1468

Closed

gerhean mentioned this pull request Mar 7, 2021

[#1468] Dashboard loading empty if base URL is given #1469

Merged

gerhean deleted the abstract-color-generator-and-sort-repo branch March 29, 2021 03:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#1420] Extract some code from summary.js #1421

[#1420] Extract some code from summary.js #1421

gerhean commented Jan 19, 2021 •

edited

Loading

gerhean Jan 19, 2021 •

edited

Loading

Tejas2805 Jan 20, 2021

gerhean Jan 20, 2021

Tejas2805 left a comment

Tejas2805 Jan 20, 2021

fzdy1914 commented Feb 7, 2021

gerhean commented Feb 15, 2021

0blivious commented Feb 15, 2021

gerhean Jan 20, 2021

gerhean commented Feb 19, 2021 •

edited

Loading

[#1420] Extract some code from summary.js #1421

[#1420] Extract some code from summary.js #1421

Conversation

gerhean commented Jan 19, 2021 • edited Loading

gerhean Jan 19, 2021 • edited Loading

Choose a reason for hiding this comment

Tejas2805 Jan 20, 2021

Choose a reason for hiding this comment

gerhean Jan 20, 2021

Choose a reason for hiding this comment

Tejas2805 left a comment

Choose a reason for hiding this comment

Tejas2805 Jan 20, 2021

Choose a reason for hiding this comment

fzdy1914 commented Feb 7, 2021

gerhean commented Feb 15, 2021

0blivious commented Feb 15, 2021

gerhean Jan 20, 2021

Choose a reason for hiding this comment

gerhean commented Feb 19, 2021 • edited Loading

gerhean commented Jan 19, 2021 •

edited

Loading

gerhean Jan 19, 2021 •

edited

Loading

gerhean commented Feb 19, 2021 •

edited

Loading