Add three build profiles and infrastructure for their toml config #440
Conversation
| } | ||
| BuildProfile::Dev => { | ||
| // Plain cargo builds use the dev cargo profile, which includes | ||
| // debug info by default. |
There was a problem hiding this comment.
This reminds me that we really should add --dev to Cargo...
fitzgen
force-pushed
the
build-profiles-toml-config
branch
2 times, most recently
from
baa523c
to
3eb5713
Compare
|
One of the new tests added depends on this: rustwasm/wasm-bindgen#1020 |
fitzgen
force-pushed
the
build-profiles-toml-config
branch
from
3eb5713
to
b73bfd1
Compare
|
This is ready for another round of review. |
fitzgen
force-pushed
the
build-profiles-toml-config
branch
from
b73bfd1
to
f3e4d62
Compare
fitzgen
force-pushed
the
build-profiles-toml-config
branch
2 times, most recently
from
74d1c72
to
a9f1ce8
Compare
|
Ok, I am pretty happy with serde and defaults stuff now. Final round of review? |
There was a problem hiding this comment.
I think I wasn't clear earlier when you said you wanted to hide the unwraps, but just to confirm you didn't want to take the strategy where there's one struct that has a bunch of non-Option fields and one that has Option and the former implement deserialize in terms of the latter?
Also just to make sure I don't forget, we should file an issue afterwards to handle unknown keys to issue warnings on things like typos. Cargo uses https://crates.io/crates/serde_ignored and wasm-pack can probably use it as well!
|
|
||
| /// Get this profile's configured `[wasm-bindgen.debug-js-glue]` value. | ||
| pub fn wasm_bindgen_debug_js_glue(&self) -> bool { | ||
| self.wasm_bindgen.debug_js_glue.unwrap() |
There was a problem hiding this comment.
Oh I thought you were leaning towards a strategy that removes these unwraps?
There was a problem hiding this comment.
I leaned back towards them because making the two-struct approach work with nested structs involves a lot of boilerplate and it isn't totally clear how we would know which defaults to use in the nested struct deserialization where we no longer know which profile we are deserializing for.
There was a problem hiding this comment.
Ok! Just wanted to confirm. We can always tweak over time if necessary!
Filed #443 |
fitzgen
force-pushed
the
build-profiles-toml-config
branch
from
a9f1ce8
to
64d52d8
Compare
Fixes #153
Fixes #160