Skip to content

Comments

feat: add verbose and extra-args to dfx build#347

Closed
chenyan-dfinity wants to merge 1 commit intomasterfrom
build
Closed

feat: add verbose and extra-args to dfx build#347
chenyan-dfinity wants to merge 1 commit intomasterfrom
build

Conversation

@chenyan-dfinity
Copy link
Contributor

No description provided.

@chenyan-dfinity chenyan-dfinity requested a review from a team as a code owner January 30, 2020 05:49
.help(UserMessage::BuildArgs.to_str()),
)
.arg(
Arg::with_name("verbose")
Copy link
Contributor

Choose a reason for hiding this comment

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

Verbosity should be a global flag. Also, this code is most likely to be wiped out when we start using slog, so why not do a PR now to do that instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verbosity should be a global flag.

I agree. Do you prefer to put the flag to the global scope with only build supporting it, or promote it to global after we have verbose mode for most commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, this code is most likely to be wiped out when we start using slog, so why not do a PR now to do that instead?

This again seems to be a global change, which should be done in a dedicated PR. I don't see an urgency to switch to slog, but I do see the urgency to enable verbose mode on build.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can start using slog here honestly. It doesn't have to be global. @chenyan-dfinity let's sync on this.

.help(UserMessage::SkipFrontend.to_str()),
)
.arg(
Arg::with_name("extra-args")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should separate those between where they're used. extra-args has no semantic meaning (moc-args? compiler-args?). We're also building more and more of a Motoko-specific build system here, and I don't agree with that direction. We can chat offline.

Copy link
Contributor Author

@chenyan-dfinity chenyan-dfinity Jan 30, 2020

Choose a reason for hiding this comment

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

As discussed on slack, the proper way is to add flag into a config file like dfx.json. Before we have the design, it would be nice to have an CLI flag to pass in extra flags. I'm fine rename it to moc-args, but the intension is to provide a way to pass in extra arguments to any compilers. We can also add a comment in the user message warning that this flag is experimental and will go away.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any update on this? Let's place discussions/more details online.

I agree with the compiler arguments flag. I don't like the verbosity change -- or maybe I am missing something. I think we can use slog just in build instead, and then fully integrate it piece by piece.

@chenyan-dfinity chenyan-dfinity deleted the build branch April 9, 2020 15:04
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.

3 participants