[WIP] Remote Image #67
Conversation
czystyl
commented
Feb 9, 2019
•
czystyl
commented
- Add httpkit
- Get image from url
- Show image
- Clean up the mess :D
czystyl
force-pushed
the
feat/remote-image
branch
from
9775f26
to
6c390f8
Compare
| @@ -18,13 +18,22 @@ | |||
| "devDependencies": { | |||
| "@opam/merlin": "^3.2.2", | |||
| "@opam/merlin-lsp": "*", | |||
| "ocaml": "~4.7.1000" | |||
| "ocaml": "~4.7.1000", | |||
| "@opam/httpkit": "*", | |||
There was a problem hiding this comment.
This is a wrong place to add dependencies.
- It's a monorepo config, so anything that depends on
brisk-macoswill fail to find those dependencies - It's
devDependencies- meaning they are either for tooling like merlin, or some local deps you'd use for tests, they will not be available in runtime
We'll have a detailed design doc and contributing guide, but for now:
- Add those deps to the
brisk-macos - Also add the resolutions to the relevant example apps (we have just components for now)
| }, | ||
| "resolutions": { | ||
| "@brisk/core": "link-dev:./core", | ||
| "@brisk/macos": "link-dev:./renderer-macos", | ||
| "@opam/dune": "ocaml/dune:dune.opam#01b46a3", | ||
| "@opam/merlin-lsp": "ocaml/merlin:merlin-lsp.opam#3e34bb5", | ||
| "brisk-reconciler": "briskml/brisk-reconciler#fa605f1" | ||
| "brisk-reconciler": "briskml/brisk-reconciler#fa605f1", | ||
| "@opam/httpkit": "ostera/httpkit:httpkit.opam#322ca26", |
There was a problem hiding this comment.
Whatever you add to the resolutions, try to keep order alphabetical (scopes go first).
There was a problem hiding this comment.
I'm opposed to this rule. Any rule you cannot automate is just mental overhead!
EDIT: And it's all about how you can justify that mental overhead.
There was a problem hiding this comment.
I'd prefer to sort them by significance but that's what would be a mental overhead, so alphabetical seems like a good compromise. (most package managers do it automatically, maybe one day we'll get that for esy)
It's not like we'll add dependencies on every single PR.
There was a problem hiding this comment.
I'm still opposed to it - I'll never remember/care about such artificial rule.
EDIT: I might even switch order on purpose 😂
EDIT2: re: "by significance". I could come up with an equally artificial rule - add the new one always on the bottom so that we know the order of new dependencies! And it would give as much value (i.e. none)
There was a problem hiding this comment.
Aren't there enough artificial rules in this world 😄 ?
There was a problem hiding this comment.
Ok, let's postpone the manual enforcing. We'll come up with a way to automate it.
There was a problem hiding this comment.
Don't fight guys :D Ok, what's about to keep the opam packages at the top and the rest of packages alphabetical?
There was a problem hiding this comment.
That’s what I’d usually do, but then there are scoped npm packages that mess up this logic.
There was a problem hiding this comment.
Also, esy might eventually switch to scoping all npm packages with @npm/, same as with @opam/.
| @@ -18,8 +18,8 @@ targets: | |||
| - Sources | |||
| settings: | |||
| HEADER_SEARCH_PATHS: '$(STDLIB_PATH)' | |||
| LIBRARY_SEARCH_PATHS: '$(STDLIB_PATH)' | |||
| OTHER_LDFLAGS: ' -framework Cocoa' | |||
| LIBRARY_SEARCH_PATHS: '$(STDLIB_PATH) /usr/local/lib/' | |||
There was a problem hiding this comment.
What's depending on gmp?
Unless we want to tell users to install it separately, we'll need to depend on https://github.com/esy-packages/esy-gmp. I'll fix it to give us cflags/libs variables.
There was a problem hiding this comment.
@rauanmayemir zarith, a transitive dependency of httpkit.
