Commented out Code

Conventions:

If in doubt, try to keep commented out code from staying within the repository. If you must, include explanatory comments as to why you have included the commented out code within the codebase.

In terms of work done on minimising commented out code within the repository:

• An issue was opened to address this problem
• A pull request was opened to resolve said issue.

Note that the line numbers referenced within this wiki page, and the related issue/PR, were relevant at the time they were created, and may have since been changed due to other contributions within the repository.

As such, these lines should be cross-referenced with the 'Files Changed' tab of the PR here, and nothing else.

Also note that the lines referenced in the issue may slightly differ from the lines referenced in the PR, again due to external contributions shifting the lines.

---

The following resolutions were reached after analysis:

• initializeGraph.ts : line 42

  • line 42: The function name arguably provides enough explanation of the purpose of the function already. Thus the explanatory comment can be removed. The sendOutput() call comment can probably be left however, as it implies the purpose of the sendOutput() function.

• abstractGraphing.ts : line 15-17

  • line 15-17: Appears to be a call to time the length of a previously written function for sorting through commits, that has now been replaced with the sortCommits() function. This can thus be removed.

• authenticate.ts : line 4-5, line 222

  • line 4-5 When uncommented, the first two import statement come up as unused. Thus, these can be deleted from the project.
  • line 222: redirectToHomePage() is only called by confirmSignOut() for the time being, and thus presumably exists solely for that purpose for now. LogInAfterConfirm() is irrelevant to being kept here, as if it were uncommented, every time the user logged out, they would automatically be logged back in, if they had saved credentials. I therefore believe it should be removed, and if in the future the redirectToHomePage() functionality responsibilities expand, then this can be reinstated.

• color.ts : line 4-8

  • line 4-8: These lines point towards a potential alternate implementation of the changeColor() function, involving the creation of various colour schemes. As this is not currently under usage, and could act as a potential point of future work, it is not a relevant comment, and can thus be removed, alongside the necessary import.

• git.ts : line 129-141, line 258-261, line 393, line 424

  • line 129-141: This commented out code appears to be either an initial attempt or an alternative attempt at the functionality of getAllCommits() that already exists. As it appears to be a subset of the functionality in getAllCommits() currently, and doesn’t introduce anything new, it may be deleted.
  • line 258-261: functionality here already covered by call to refreshAll() in line 240 of git.ts
  • line 393: this line is already covered by the following invocation to Git.Reference.nameToId
  • line 424: this is also seemingly covered by Git.Reference.nameToId

• graphing.ts : line 77-79

  • line 77-79: sortCommits() is always called prior to populateCommits(). Thus - as before - the code in this commented section is already encapsulated in sortedCommits(), and is thus not needed here, and can therefore be removed.

• graphSetup.ts : line 40, line 142-144, line 231, line 236, line 281-289

  • line 40: This is a line specifying the direction of smoothing for lines, and is horizontal by default for some situations. As it is very specific, and seems unnecessary, it may be removed. Further documentation can be found here under smooth.forceDirection
  • line 142-144: given the explanation of this, and the supporting method initGraphInBg(), these comments can probably be left as a todo implementation.
  • line 231: This seems to be made redundant by network.fit(moveOptions);
  • line 236: This also seems to be made redundant by network.fit(moveOptions);
  • line 281-289: These seem to be important to keep, as it implies the usage of the addBranchestoNode() method (which has no other reference yet). Indentation for this can be cleaned up however.

• images.ts : line 34-40

  • line 34-40: this code block has a comment explaining why it was left in, in the first place. Seeing as a new issue has not yet been opened for the described problem, it is probably best to leave this code block in, for the sake of whoever does pick it up, in the future.

• repo.ts : line 556-576, line 579-580

  • line 556-576: the handleModal() function seems to already be covered by the “close” class and the “data-dismissal” attributes placed on the close button within all modals. initModal() also seems redundant, as all of the modals are already existent within the header.component.html. One can assume then that this was an attempt at an alternate implementation, where modals could be created on the fly, and with input text as a part of their creation. As this approach hasn’t been used at all, it’s probably best to remove it, and clean up the file further. Note as a part of removing both of these methods, both of the calls in displayModal() should be removed.
  • line 579-580: These comments can’t be found unfortunately, and a decision can therefore not be made on them.

• style.css : line 141-151, line 161-163

  • line 141-151: While these stylings aren’t being used, they can probably be deleted, and re-introduced if needed. They otherwise simply unnecessarily take up space.
  • line 161-163: Same reasoning here as above - if the problem is re-introduced, a fix can be simply found by identifying the relevant component, and applying margin shifts as needed.