Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(83)

Issue 29367148: Issue 4720 - Edge packager does not support devenv (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 11 months ago by Oleksandr
Modified:
2 years, 1 month ago
Reviewers:
kzar, Sebastian Noack
CC:
Vasily Kuznetsov
Visibility:
Public.

Description

Issue 4720 - Edge packager does not support devenv

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Rebase #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -1 line) Patch
M build.py View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M packagerEdge.py View 1 2 1 chunk +7 lines, -0 lines 1 comment Download

Messages

Total messages: 10
Oleksandr
2 years, 11 months ago (2016-12-09 06:23:24 UTC) #1
kzar
LGTM
2 years, 11 months ago (2016-12-12 12:08:36 UTC) #2
Sebastian Noack
Note that chromeDevenvPoller__.js which is included when using devenv is using chrome.runtime.reload(). Does that API ...
2 years, 11 months ago (2016-12-13 14:37:34 UTC) #3
Oleksandr
On 2016/12/13 14:37:34, Sebastian Noack wrote: > Note that chromeDevenvPoller__.js which is included when using ...
2 years, 11 months ago (2016-12-13 16:29:33 UTC) #4
Oleksandr
On 2016/12/13 14:37:34, Sebastian Noack wrote: > Note that chromeDevenvPoller__.js which is included when using ...
2 years, 11 months ago (2016-12-13 16:29:33 UTC) #5
Sebastian Noack
On 2016/12/13 16:29:33, Oleksandr wrote: > chrome.runtime.reload() does not exist in Edge, however it is ...
2 years, 11 months ago (2016-12-14 12:23:46 UTC) #6
Sebastian Noack
On 2016/12/14 12:23:46, Sebastian Noack wrote: > On 2016/12/13 16:29:33, Oleksandr wrote: > > chrome.runtime.reload() ...
2 years, 1 month ago (2017-09-23 18:25:22 UTC) #7
Oleksandr
runtime.reload() is supported now indeed. Rebased.
2 years, 1 month ago (2017-09-25 08:26:24 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py#newcode191 packagerEdge.py:191: if devenv: This logic is duplicated in packagerChrome.py. Can ...
2 years, 1 month ago (2017-09-25 15:58:53 UTC) #9
Oleksandr
2 years, 1 month ago (2017-10-13 14:33:47 UTC) #10
On 2017/09/25 15:58:53, Sebastian Noack wrote:
> https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py
> File packagerEdge.py (right):
> 
>
https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py#new...
> packagerEdge.py:191: if devenv:
> This logic is duplicated in packagerChrome.py. Can you moveit into a function
> and reuse that function here?
> 
> Also is it intentional that you left out the qunit test suite?

No, it was unintentional that I left out qunit test suite. Since Tristan is
going to take this over, and basically the review would look different after
reusing the code in packagerChrome.py I will close this codereview in favor of
whatever Tristan will submit.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5