[Webkit-unassigned] [Bug 153818] Format revisions for display according to repository type

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=153818

--- Comment #6 from Jason Marcell <jmarcell at apple.com> ---
Comment on attachment 270610
  --> https://bugs.webkit.org/attachment.cgi?id=270610
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270610&action=review

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:127
>> +            linkElement.textContent = this._formatRevisionForDisplay(commit.revisionNumber, branch.repository);
> 
> Is "revision number" the right terminology for git?
> 
> Just curious, no need to update all the naming for this patch even if it's not quite right.

I've thought about that before. With the introduction of Git, the revision is a hash, which you might argue is not a number because we're working with it in string form everywhere. On the other hand, a Git hash is actually a hexadecimal number. I think maybe the term "revision" instead of "revision number" might be more appropriate.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:339
>> +    _formatRevisionForDisplay: function (revision, repository)
> 
> Please remove the space after "function".

Will do.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:347
>> +            console.assert(false, "Should not get here; " + repository.name + " did not specify a known VCS type.");
> 
> WebKit style is to not use else after return. Here is how I would write this:
> 
> console.assert(repository.isSVN || repository.isGit);
> if (repository.isSVN)
>     return "r" + revision;
> // Truncating for display. Git traditionally uses seven characters for a short hash.
> return revision.substr(0, 7);

Thanks. I'll use this.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160204/7f4c11e4/attachment.html>