dependencies - Issue 5924365505921024: Use ensure_dependencies.py instead of subrepositories

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Delta Between Two Patch Sets: dependencies

Issue 5924365505921024: Use ensure_dependencies.py instead of subrepositories (Closed)

Left Patch Set: Created March 13, 2015, 8:55 p.m.

Right Patch Set: Use buildtools transitively Created March 17, 2015, 2:47 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 _root = hg:https://bitbucket.org/adblockplus/	1 _root = hg:https://bitbucket.org/adblockplus/

2 _self = buildtools/ensure_dependencies.py	2 _self = adblockplus/buildtools/ensure_dependencies.py

3 buildtools = buildtools hg:d06704a466c5
Wladimir Palant 2015/03/13 21:02:55 Note that you might want to reuse the buildtools r Note that you might want to reuse the buildtools repository within adblockplus instead - it's the right revision to build Adblock Plus. Felix Dahlke 2015/03/15 23:01:54 That's what I'm doing actually - only including th Show quoted text On 2015/03/13 21:02:55, Wladimir Palant wrote: > Note that you might want to reuse the buildtools repository within adblockplus > instead - it's the right revision to build Adblock Plus. That's what I'm doing actually - only including this here since it seems ensure_dependencies.py needs this for self updating. Could I remove this line and change the one above to this: _self = adblockplus/adblockplus/buildtools/ensure_dependencies.py and it'd self update properly? Whether or not it works - I don't think it's a good idea to depend on a particular transitive dependency's existence here, so I'd rather keep it as it is or remove _self (seems to work, disabling self updating though). Wladimir Palant 2015/03/16 11:59:33 Yes, that's what you should do. You can always tes Show quoted text On 2015/03/15 23:01:54, Felix H. Dahlke wrote: > Could I remove this line and change the one above to this: > > _self = adblockplus/adblockplus/buildtools/ensure_dependencies.py > > and it'd self update properly? Yes, that's what you should do. You can always test the self-update by making a minimal change to your ensure_dependencies.py - it should revert that change on a dependency update. Show quoted text > Whether or not it works - I don't think it's a good idea to depend on a > particular transitive dependency's existence here, so I'd rather keep it as it > is or remove _self (seems to work, disabling self updating though). There is a reason why we use _self - people will forget to update ensure_dependencies.py otherwise. Note that if adblockplus/adblockplus/buildtools doesn't exist then nothing will fail - there will be merely a warning. So IMHO definitely better than having an additional dependency for no good reason or disabling self-update altogether. Felix Dahlke 2015/03/17 14:49:09 I still think it makes more sense to depend on bui Show quoted text On 2015/03/16 11:59:33, Wladimir Palant wrote: > On 2015/03/15 23:01:54, Felix H. Dahlke wrote: > > Could I remove this line and change the one above to this: > > > > _self = adblockplus/adblockplus/buildtools/ensure_dependencies.py > > > > and it'd self update properly? > > Yes, that's what you should do. You can always test the self-update by making a > minimal change to your ensure_dependencies.py - it should revert that change on > a dependency update. > > > Whether or not it works - I don't think it's a good idea to depend on a > > particular transitive dependency's existence here, so I'd rather keep it as it > > is or remove _self (seems to work, disabling self updating though). > > There is a reason why we use _self - people will forget to update > ensure_dependencies.py otherwise. Note that if > adblockplus/adblockplus/buildtools doesn't exist then nothing will fail - there > will be merely a warning. So IMHO definitely better than having an additional > dependency for no good reason or disabling self-update altogether. I still think it makes more sense to depend on buildtools directly for that, but I don't have a particularly strong opinion. Going with using the one required by adblockplus now - less stuff in the root directory at least :)
4 adblockplus = adblockplus hg:ec627ace494e	3 adblockplus = adblockplus hg:ec627ace494e

LEFT	RIGHT