Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(125)

Issue 672363002: Make skydb load examples/home.sky by default. (Closed)

Created:
6 years, 2 months ago by eseidel
Modified:
6 years, 1 month ago
CC:
abarth-chromium, Aaron Boodman, ben+mojo_chromium.org, darin (slow to review), esprehn, mojo-reviews_chromium.org, ojan, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Visibility:
Public.

Description

Make skydb load examples/home.sky by default. I had to also register for the text/plain mime-type which turned out to be harder than expected. Mostly due to my confusion with mojo_shell using last-argument-wins argument parsing. R=ianh@google.com Committed: https://chromium.googlesource.com/external/mojo/+/ff945ef78a6c1955a5ccb2c09d73f4d6f028c515

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -3 lines) Patch
M mojo/shell/dynamic_application_loader.cc View 1 chunk +3 lines, -0 lines 1 comment Download
M sky/examples/home.sky View 1 chunk +2 lines, -0 lines 1 comment Download
M sky/tools/debugger/prompt/prompt.cc View 1 chunk +6 lines, -2 lines 1 comment Download
M sky/tools/skydb View 1 chunk +8 lines, -1 line 1 comment Download

Messages

Total messages: 12 (2 generated)
eseidel
Ready for review.
6 years, 2 months ago (2014-10-23 21:53:54 UTC) #2
Hixie
https://codereview.chromium.org/672363002/diff/1/sky/examples/home.sky File sky/examples/home.sky (right): https://codereview.chromium.org/672363002/diff/1/sky/examples/home.sky#newcode2 sky/examples/home.sky:2: <sky> booo hissss LGTM.
6 years, 2 months ago (2014-10-23 21:54:54 UTC) #3
Hixie
6 years, 2 months ago (2014-10-23 21:54:56 UTC) #4
Hixie
https://codereview.chromium.org/672363002/diff/1/sky/tools/debugger/prompt/prompt.cc File sky/tools/debugger/prompt/prompt.cc (right): https://codereview.chromium.org/672363002/diff/1/sky/tools/debugger/prompt/prompt.cc#newcode51 sky/tools/debugger/prompt/prompt.cc:51: } why curlies?
6 years, 2 months ago (2014-10-23 21:55:23 UTC) #5
Hixie
https://codereview.chromium.org/672363002/diff/1/sky/tools/skydb File sky/tools/skydb (right): https://codereview.chromium.org/672363002/diff/1/sky/tools/skydb#newcode16 sky/tools/skydb:16: SUPPORTED_MIME_TYPES = [ this is temporary, right? Long term ...
6 years, 2 months ago (2014-10-23 21:56:21 UTC) #6
eseidel
Committed patchset #1 (id:1) manually as ff945ef78a6c1955a5ccb2c09d73f4d6f028c515 (presubmit successful).
6 years, 2 months ago (2014-10-23 21:56:48 UTC) #7
abarth-chromium
lgtm
6 years, 2 months ago (2014-10-23 21:57:05 UTC) #8
eseidel
Correct, temporary.
6 years, 2 months ago (2014-10-23 22:01:13 UTC) #9
Aaron Boodman
https://codereview.chromium.org/672363002/diff/1/mojo/shell/dynamic_application_loader.cc File mojo/shell/dynamic_application_loader.cc (right): https://codereview.chromium.org/672363002/diff/1/mojo/shell/dynamic_application_loader.cc#newcode178 mojo/shell/dynamic_application_loader.cc:178: << "INSECURELY LOADING AS EXECUTABLE CODE INSTEAD" << std::endl; ...
6 years, 1 month ago (2014-10-31 16:14:49 UTC) #11
eseidel
6 years, 1 month ago (2014-10-31 16:47:06 UTC) #12
Message was sent while issue was closed.
I believe I added that so that it was more clear what the default behavior was. 
I got the mime type wrong or something during testing and was confused why mojo
was just crashing instead of launching sky iirc.

It's true though... the current codepath just tries to load/execute any random
file mojo doesn't otherwise understand.

Powered by Google App Engine
This is Rietveld 408576698