Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add toml like configuration #194

Closed
wants to merge 2 commits into from
Closed

Conversation

csmoe
Copy link
Member

@csmoe csmoe commented Jun 21, 2018

Address #160

Config format: (from #160 (comment)):

[package.metadata.wasm-pack.build.<build_mode>]
debug = true
features = ["console_error_panic_hook"]
rustc-opt-level = 0
wasm-opt = false
wasm-snip = false
wasm-bindgen = true

build_mode = debug | profiling | production

@ashleygwilliams ashleygwilliams added the work in progress do not merge! label Jun 22, 2018
@ashleygwilliams
Copy link
Member

this is an awesome start @csmoe let me know if you need any help! i think the plan for landing this should be coordinated with the work in #163 and we'll need to get the other commands in there as well. i can start on that (or get others to start on that!) soon!

@mgattozzi
Copy link
Contributor

@csmoe when you feel it's good for review just ping us :) I also just wanted to say I'm super stoked for this so thank you for working on it.

@csmoe csmoe force-pushed the config branch 2 times, most recently from 65a8712 to cc42c6e Compare June 28, 2018 10:53
@ashleygwilliams ashleygwilliams added this to the 0.5.0 milestone Jul 13, 2018
@fitzgen
Copy link
Member

fitzgen commented Jul 13, 2018

Is this actually still WIP? Or waiting on review?

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether the manifest module existed at the time this PR was written or not, but that seems to me like the "proper" location for this configuration parsing code.

@csmoe
Copy link
Member Author

csmoe commented Jul 14, 2018

@fitzgen those changes were introduced into manifest locally indeed, but I don't know how to test for them since there is no build cmd, maybe I can take care of the splitting stuff, could you take a look at the no-response comment?#188 (comment)

@ashleygwilliams
Copy link
Member

@csmoe if i am correct this is waiting on #216 ? if so, you can rebase onto that branch and then change the PR to be into that PR so we can be ready to merge when that lands (ideally today). thank you so much for all your hard work and especially for your patience!

@ashleygwilliams ashleygwilliams added needs tests please add tests to this PR needs docs please add docs to this PR labels Jul 26, 2018
@ashleygwilliams
Copy link
Member

@csmoe if i am correct this is waiting on #216 ? if so, you can rebase onto that branch and then change the PR to be into that PR so we can be ready to merge when that lands (ideally today). thank you so much for all your hard work and especially for your patience!

additionally, i think we'll want some very basic docs and tests for this. lemme know if you want some direction on that.

@csmoe
Copy link
Member Author

csmoe commented Jul 26, 2018

@ashleygwilliams yes, I'm waiting for build.
will restart tomorrow since it's midnight in my timezone.

@ashleygwilliams ashleygwilliams modified the milestones: 0.5.0, 0.6.0 Jul 26, 2018
for (key, value) in table {
match (key.as_str(), value.clone()) {
("debug", Value::Boolean(b)) => wasm_pack_config.debug = From::from(b),
("features", Value::Array(a)) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does features work?
@ashleygwilliams impl From<Features> for CargoTomlFeaturesSection?

@csmoe csmoe mentioned this pull request Aug 2, 2018
@csmoe
Copy link
Member Author

csmoe commented Nov 10, 2018

@ashleygwilliams seems suppressed by #440, so close this?

@csmoe csmoe closed this Nov 12, 2018
@ashleygwilliams ashleygwilliams removed blocked needs docs please add docs to this PR needs tests please add tests to this PR work in progress do not merge! labels Jan 11, 2019
@ashleygwilliams ashleygwilliams removed this from the 0.6.0 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants