<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Format revisions for display according to repository type"
href="https://bugs.webkit.org/show_bug.cgi?id=153818#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Format revisions for display according to repository type"
href="https://bugs.webkit.org/show_bug.cgi?id=153818">bug 153818</a>
from <span class="vcard"><a class="email" href="mailto:jmarcell@apple.com" title="Jason Marcell <jmarcell@apple.com>"> <span class="fn">Jason Marcell</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=270610&action=diff" name="attach_270610" title="Patch">attachment 270610</a> <a href="attachment.cgi?id=270610&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=270610&action=review">https://bugs.webkit.org/attachment.cgi?id=270610&action=review</a>
<span class="quote">>> 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.</span >
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.
<span class="quote">>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotQueueView.js:339
>> + _formatRevisionForDisplay: function (revision, repository)
>
> Please remove the space after "function".</span >
Will do.
<span class="quote">>> 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);</span >
Thanks. I'll use this.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>