Issue 29953557: Issue 7145 - Change chrome extension name for cws (Closed)
DescriptionIssue 7145 - Change chrome extension name for cws
Patch Set 1 #
Total comments: 2
MessagesTotal messages: 7
https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome#new... metadata.chrome:34: name = __MSG_name_chrome__ So this string already exists somewhere (where?), and doesn't need to be added at the same time as we make this change?
https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome#new... metadata.chrome:34: name = __MSG_name_chrome__ On 2018/11/28 16:01:34, kzar wrote: > So this string already exists somewhere (where?), and doesn't need to be added > at the same time as we make this change? Yes and yes. For the string see #7146. I would say it's best we include this in the dependency update that would land #7146. If you prefer we could also land a separate dependency update with just 7146/7145 but that would be more work for us (one additional dependency update)?
On 2018/11/28 16:10:58, wspee wrote: > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome > File metadata.chrome (right): > > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome#new... > metadata.chrome:34: name = __MSG_name_chrome__ > On 2018/11/28 16:01:34, kzar wrote: > > So this string already exists somewhere (where?), and doesn't need to be added > > at the same time as we make this change? > > Yes and yes. For the string see #7146. I would say it's best we include this in > the dependency update that would land #7146. If you prefer we could also land a > separate dependency update with just 7146/7145 but that would be more work for > us (one additional dependency update)? i.e. add 7146 to 6936 and land 7145 afterwards.
On 2018/11/28 16:17:43, wspee wrote: > On 2018/11/28 16:10:58, wspee wrote: > > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome > > File metadata.chrome (right): > > > > > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome#new... > > metadata.chrome:34: name = __MSG_name_chrome__ > > On 2018/11/28 16:01:34, kzar wrote: > > > So this string already exists somewhere (where?), and doesn't need to be > added > > > at the same time as we make this change? > > > > Yes and yes. For the string see #7146. I would say it's best we include this > in > > the dependency update that would land #7146. If you prefer we could also land > a > > separate dependency update with just 7146/7145 but that would be more work for > > us (one additional dependency update)? > > i.e. add 7146 to 6936 and land 7145 afterwards. I see. In my opinion, we should add the new string to adblockplusui first (#7146), then make the change here at the same time as updating the adblockplusui dependency (#7145). Therefore I think that #7145 should be marked as blocked by #7146 in Trac (currently it's the other way around). What do you think?
On 2018/11/30 12:45:49, kzar wrote: > On 2018/11/28 16:17:43, wspee wrote: > > On 2018/11/28 16:10:58, wspee wrote: > > > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome > > > File metadata.chrome (right): > > > > > > > > > https://codereview.adblockplus.org/29953557/diff/29953558/metadata.chrome#new... > > > metadata.chrome:34: name = __MSG_name_chrome__ > > > On 2018/11/28 16:01:34, kzar wrote: > > > > So this string already exists somewhere (where?), and doesn't need to be > > added > > > > at the same time as we make this change? > > > > > > Yes and yes. For the string see #7146. I would say it's best we include this > > in > > > the dependency update that would land #7146. If you prefer we could also > land > > a > > > separate dependency update with just 7146/7145 but that would be more work > for > > > us (one additional dependency update)? > > > > i.e. add 7146 to 6936 and land 7145 afterwards. > > I see. In my opinion, we should add the new string to adblockplusui first > (#7146), then make the change here at the same time as updating the > adblockplusui dependency (#7145). Therefore I think that #7145 should be marked > as blocked by #7146 in Trac (currently it's the other way around). What do you > think? SGTM, I have updated 7145/7146 accordingly.
On 2018/11/30 14:39:07, wspee wrote: > SGTM, I have updated 7145/7146 accordingly. Nice one. When Sebastian gets back we can triage #7145, but in the mean time you guys can go ahead with #7146. 
|
|||||||||||||||||||||||||||||||||||||||||||||||
