Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29630668: Issue 6142 - suppress missing Node.js warning when passing -q (Closed)

Dec. 5, 2017, 1:58 p.m. by tlucas
Dec. 14, 2017, 11:16 a.m.
Sebastian Noack


Issue 6142 - suppress missing Node.js warning when passing -q Base revision: https://hg.adblockplus.org/buildtools/file/d9e8c5035624

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M ensure_dependencies.py View 1 chunk +2 lines, -2 lines 1 comment Download


Total messages: 5
Patch Set 1: Suppress "missing Node.js" warning when passing -q https://codereview.adblockplus.org/29630668/diff/29630669/ensure_dependencies.py File ensure_dependencies.py (right): https://codereview.adblockplus.org/29630668/diff/29630669/ensure_dependencies.py#newcode440 ...
Dec. 5, 2017, 2 p.m. (2017-12-05 14:00:12 UTC) #1
Vasily Kuznetsov
On 2017/12/05 14:00:12, tlucas wrote: > Patch Set 1: > > Suppress "missing Node.js" warning ...
Dec. 5, 2017, 2:01 p.m. (2017-12-05 14:01:50 UTC) #2
For reference, this issue got rejected.
Dec. 14, 2017, 10:37 a.m. (2017-12-14 10:37:29 UTC) #3
On 2017/12/14 10:37:29, tlucas wrote: > For reference, this issue got rejected. Out of curiosity: ...
Dec. 14, 2017, 11:04 a.m. (2017-12-14 11:04:35 UTC) #4
Dec. 14, 2017, 11:16 a.m. (2017-12-14 11:16:17 UTC) #5
Message was sent while issue was closed.
On 2017/12/14 11:04:35, f.nicolaisen wrote:
> On 2017/12/14 10:37:29, tlucas wrote:
> > For reference, this issue got rejected.
> Out of curiosity: By who, and with what reasoning?


3:17 PM <tlucas> snoack: regarding webpack: yes, but that's only used on the
actual build-server - which does have Node.js installed
3:22 PM <snoack> tlucas: why does these servers, which don't use parts of
sitescripts that rely on buildtools, run ensure_dependencies.py in the first
place then? Couldn't we just not run ensure_depdencies.py there?
3:23 PM <~palant> snoack: "not relying on buildtools" is assumptions. the
correct way to get the code is to check it out and run ensure_dependencies.py.
3:26 PM <~palant> tlucas: snoack: maybe it's time to split up sitescripts. it's
currently a collection of largely unrelated scripts, some kind of split might
make more sense.      <--- this is what we did in the end
3:26 PM <tlucas> palant: snoack : that what the document is about - it's just
not feasible for "right now"
3:26 PM <~palant> tlucas: snoack: like we split out the CMS a while ago.
3:26 PM <tlucas> palant:
3:26 PM <~palant> tlucas: ah, I see :)
3:27 PM <tlucas> vasily: ^ in case you want to join all this :D
3:28 PM <snoack> palant: Do you have any better idea than just silencing the
error on the server?
3:31 PM <~palant> snoack: tlucas: as things are now, npm is a requirement for
sitescripts. silencing the error is a bad idea IMHO - we should rather make sure
npm is installed on those servers and hope that we can have to long-term
solution sooner rather than later.
3:32 PM <snoack> palant: Sounds reasonable to me. (CC: tlucas)
3:32 PM <tlucas> snoack, palant : i agree it's time to split out the part in
question, but i don't think postponing any creation of nightlies until this has
landed is a good idea. Also, If servers try to use anythin Node.js related where
Node.js is not installed, there is a traceback (and hence a meaningful message
from cron), giving the ops the opportunity to actually handle things
3:32 PM <tlucas> palant: so you are suggesting to install Node.js on every
server currently using sitescripts?
3:32 PM <tlucas> ferris: ^
3:33 PM <~palant> tlucas: yes, that's what I am suggesting - that's the only
correct thing to do IMHO. it
3:34 PM <~palant> tlucas: it's not like installing Node will keep those servers
busy for more than a minute or occupy lots of space.
3:35 PM <tlucas> palant: it would - from our side - definitely be the least
troublesome solution, yes..
3:35 PM <tlucas> palant: would you mind adding a note for this to the
3:36 PM <~palant> tlucas: which one is that? I don't see a link to it anywhere.
3:36 PM <tlucas> palant: http://hub.eyeo.com/issues/5876

and in hub:


Powered by Google App Engine
This is Rietveld