Create Individual EWS Pages

• Component: New Bugs
• Status: RESOLVED FIXED

[obinna obike]

Created Individual EWS Pages

[obinna obike]

Created attachment 315961 [details]
Reverted the changes to urls.py and bugzilla.py

I changed the url in the urls.py back to wekbkit server instead of my local server. I also reverted the changes to bugzilla.py.

[Aakash Jain]

Comment on attachment 315961 [details]
Reverted the changes to urls.py and bugzilla.py

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

> Tools/ChangeLog:7
> +

Please describe your changes in the ChangeLog. Please see https://webkit.org/contributing-code/#changelog-files

> Tools/QueueStatusServer/handlers/patch.py:50
> +                    per_queue_statuses = queue_status.get(status.queue_name, [])

As I requested couple of times in-person, please avoid this code duplication. It's a simple if else, you should be able to avoid the duplication.

> Tools/QueueStatusServer/handlers/statusbubble.py:91
> +                message = "\n\nhttps://build.webkit.org/dashboard/"

This is completely unrelated change. Please remove this change from this patch and create a separate bug for this change. Make sure you test it as well.

[Aakash Jain]

*** Bug 174728 has been marked as a duplicate of this bug. ***

[obinna obike]

Created attachment 317149 [details]
status bubble link to individual EWS page.

[Build Bot]

Attachment 317149 [details] did not pass style-queue:


ERROR: Tools/QueueStatusServer/handlers/patch.py:58:  no newline at end of file  [pep8/W292] [5]
ERROR: Tools/QueueStatusServer/handlers/patch.py:58:  [Patch.get] Instance of 'Patch' has no 'response' member  [pylint/E1101] [5]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Aakash Jain]

Comment on attachment 317149 [details]
status bubble link to individual EWS page.

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

> Tools/ChangeLog:9
> +        to go directly to the Individual EWS page.I also added made changes to the code so that if the number

"added made changes" ?

> Tools/ChangeLog:10
> +        of items in the queue is 0 then have a message that says waiting in the queue processing hasnt started.

hasnt => hasn't.
0 => zero

> Tools/ChangeLog:11
> +        I also added a link to the list of all of the EWS queues at the bottom if there is no or one EWS on the page.

"if there is no or one EWS on the page"? Wouldn't that link be always there?

> Tools/ChangeLog:15
> +        * QueueStatusServer/handlers/statusbubble.py:

Please add a small description of each change here as well.

> Tools/QueueStatusServer/handlers/patch.py:52
> +                    queue_status[status.queue_name] = per_queue_statuses

Did you see my previous comment: "As I requested couple of times in-person, please avoid this code duplication. It's a simple if else, you should be able to avoid the duplication."

[obinna obike]

Created attachment 317428 [details]
status bubble link to individual EWS page.

[Aakash Jain]

The patch looks good except for the code duplication which I requested earlier. You are not able to simplify the if-else?

[obinna obike]

Created attachment 317447 [details]
status bubble link to individual EWS page.

changed the if else statement to two if statements to simplify the code.

[Aakash Jain]

Code duplication is still there. It shouldn't be difficult to simplify it.

[Dean Johnson]

Comment on attachment 317447 [details]
status bubble link to individual EWS page.

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

> Tools/QueueStatusServer/handlers/patch.py:51
> +                queue_status[status.queue_name] = per_queue_statuses

I would recommend rewriting L41-L51 as follows:

queue_status = {}
for status in statuses:
    bug_id = status.active_bug_id  # Should be the same for every status
    per_queue_statuses = queue_status.get(status.queue_name, [])
    per_queue_statuses.append(status)
    queue_status[status.queue_name] = per_queue_statuses

# Show a single EWS status instead of all.
if queue_name is not None:  
    single_queue_status = {}
    single_queue_status[queue_name] = queue_statuses.get(status.queue_name, [])
    queue_statuses = single_queue_status

This is optimal because it allows for existing code to continue functioning as it was, and just picks out the individual status if it's requested.

[Dean Johnson]

(In reply to Dean Johnson from comment #11)
> Comment on attachment 317447 [details]
> status bubble link to individual EWS page.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317447&action=review
> 
> > Tools/QueueStatusServer/handlers/patch.py:51
> > +                queue_status[status.queue_name] = per_queue_statuses
> 
> I would recommend rewriting L41-L51 as follows:
> 
> queue_status = {}
> for status in statuses:
>     bug_id = status.active_bug_id  # Should be the same for every status
>     per_queue_statuses = queue_status.get(status.queue_name, [])
>     per_queue_statuses.append(status)
>     queue_status[status.queue_name] = per_queue_statuses
> 
> # Show a single EWS status instead of all.
> if queue_name is not None:  
>     single_queue_status = {}
>     single_queue_status[queue_name] = queue_statuses.get(status.queue_name,
> [])
Oops, this line should read:
      single_queue_status[queue_name] = queue_statuses.get(queue_name, [])
>     queue_statuses = single_queue_status
> 
> This is optimal because it allows for existing code to continue functioning
> as it was, and just picks out the individual status if it's requested.

[obinna obike]

Created attachment 317460 [details]
status bubble link to individual EWS page.

[obinna obike]

Created attachment 317581 [details]
Updated Patch

[Aakash Jain]

Patch looks good. Please do svn update and re-create the patch, to ensure it doesn't fail to apply to repository.

[obinna obike]

Created attachment 317646 [details]
Updated Patch

[obinna obike]

Created attachment 317647 [details]
Updated Patch

[Aakash Jain]

Looks good to me.

[WebKit Commit Bot]

Comment on attachment 317647 [details]
Updated Patch

Clearing flags on attachment: 317647

Committed r220434: <http://trac.webkit.org/changeset/220434>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33790314>

[Aakash Jain]

Updated the server. These changes are now live.