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

Issue 29368690: [buildtools] Issue 4578 - Make uap3:AppExtension.Id configurable for Microsoft Edge builds (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by wspee
Modified:
2 years, 7 months ago
Reviewers:
Sebastian Noack
CC:
Vasily Kuznetsov, Wladimir Palant
Visibility:
Public.

Description

[buildtools] Issue 4578 - Make uap3:AppExtension.Id configurable for Microsoft Edge builds

Patch Set 1 #

Total comments: 13

Patch Set 2 : Test AppManifest for release and devbuilds using xml xpath expressions #

Total comments: 2

Patch Set 3 : Changed test_create_appx_manifest to no longer use lxml #

Patch Set 4 : Made id configure able #

Total comments: 2

Patch Set 5 : Don't format and use extension_id_* #

Patch Set 6 : Fallback to EdgeExtension if no extension_id configured #

Total comments: 2

Patch Set 7 : Removed extension_id_devbuild option from test metadata #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -75 lines) Patch
M packagerEdge.py View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M templates/edge/AppxManifest.xml.tmpl View 1 chunk +1 line, -1 line 0 comments Download
M tests/AppManifest.xml.expect View 1 1 chunk +0 lines, -61 lines 0 comments Download
M tests/metadata.edge View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M tests/test_packagerEdge.py View 1 2 1 chunk +90 lines, -13 lines 0 comments Download

Messages

Total messages: 19
wspee
2 years, 8 months ago (2016-12-16 10:28:01 UTC) #1
Wladimir Palant
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' Given that ...
2 years, 8 months ago (2016-12-16 10:39:25 UTC) #2
Sebastian Noack
I added Vasily as reviewer, who wrote the intial code for the Edge packager, and ...
2 years, 8 months ago (2016-12-16 11:20:11 UTC) #3
Sebastian Noack
I just noticed, today, that Vasily is on vacation. So removing him again as reviewer. ...
2 years, 8 months ago (2016-12-19 15:30:20 UTC) #4
wspee
On 2016/12/16 10:39:25, Wladimir Palant wrote: > Given that this logic isn't something that is ...
2 years, 8 months ago (2016-12-19 16:07:05 UTC) #5
Sebastian Noack
Please always reply inline to inline comments, i.e. go to the patch the comment you ...
2 years, 8 months ago (2016-12-19 17:19:52 UTC) #6
wspee
https://codereview.adblockplus.org/29368690/diff/29368691/tests/AppManifest.xml.expect File tests/AppManifest.xml.expect (right): https://codereview.adblockplus.org/29368690/diff/29368691/tests/AppManifest.xml.expect#newcode44 tests/AppManifest.xml.expect:44: Id="1.0" On 2016/12/19 17:19:52, Sebastian Noack wrote: > > ...
2 years, 8 months ago (2016-12-21 14:15:52 UTC) #7
Sebastian Noack
https://codereview.adblockplus.org/29368690/diff/29368691/tests/AppManifest.xml.expect File tests/AppManifest.xml.expect (right): https://codereview.adblockplus.org/29368690/diff/29368691/tests/AppManifest.xml.expect#newcode44 tests/AppManifest.xml.expect:44: Id="1.0" On 2016/12/21 14:15:51, wspee wrote: > On 2016/12/19 ...
2 years, 8 months ago (2016-12-21 15:54:16 UTC) #8
wspee
I have updated the code to no longer depend on lxml but instead use the ...
2 years, 7 months ago (2017-01-02 14:16:19 UTC) #9
Wladimir Palant
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' On 2016/12/19 ...
2 years, 7 months ago (2017-01-05 12:20:49 UTC) #10
Sebastian Noack
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' On 2017/01/05 ...
2 years, 7 months ago (2017-01-05 12:27:57 UTC) #11
wspee
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' On 2017/01/05 ...
2 years, 7 months ago (2017-01-06 16:18:11 UTC) #12
wspee
Removed Wladimir as a reviewer as he will be afk for the next two weeks.
2 years, 7 months ago (2017-01-06 17:21:46 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' On 2017/01/06 ...
2 years, 7 months ago (2017-01-07 10:18:46 UTC) #14
wspee
https://codereview.adblockplus.org/29368690/diff/29370844/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29370844/packagerEdge.py#newcode89 packagerEdge.py:89: metadata_id = 'id_{}'.format(metadata_id_suffix) On 2017/01/07 10:18:46, Sebastian Noack wrote: ...
2 years, 7 months ago (2017-01-09 09:15:53 UTC) #15
wspee
https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py File packagerEdge.py (right): https://codereview.adblockplus.org/29368690/diff/29368691/packagerEdge.py#newcode87 packagerEdge.py:87: params['app_extension_id'] = '1.0' if release_build else 'EdgeExtension' On 2017/01/07 ...
2 years, 7 months ago (2017-01-09 13:32:29 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29368690/diff/29370870/tests/metadata.edge File tests/metadata.edge (right): https://codereview.adblockplus.org/29368690/diff/29370870/tests/metadata.edge#newcode21 tests/metadata.edge:21: extension_id_devbuild = EdgeExtension This seems redundant now, where we ...
2 years, 7 months ago (2017-01-09 14:52:19 UTC) #17
wspee
https://codereview.adblockplus.org/29368690/diff/29370870/tests/metadata.edge File tests/metadata.edge (right): https://codereview.adblockplus.org/29368690/diff/29370870/tests/metadata.edge#newcode21 tests/metadata.edge:21: extension_id_devbuild = EdgeExtension On 2017/01/09 14:52:19, Sebastian Noack wrote: ...
2 years, 7 months ago (2017-01-10 08:09:00 UTC) #18
Sebastian Noack
2 years, 7 months ago (2017-01-10 12:54:42 UTC) #19
LGTM

As usual, can you send me the patch (including the metadata,  etc.)?
Sign in to reply to this message.

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