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

Add support to define a default target in manifest file #18

Merged
merged 1 commit into from Apr 13, 2016

Conversation

ankitspd
Copy link
Member

@@ -805,6 +807,15 @@ bool BuildSystemImpl::build(StringRef target) {
// Create the execution queue.
executionQueue = delegate.createExecutionQueue();

// If target name is not passed then we try to load the default target name
// from manifest file
if (target.size() < 1) {
Copy link
Member

Choose a reason for hiding this comment

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

You can use target.empty() (and likewise two lines below).

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@ddunbar
Copy link
Member

ddunbar commented Apr 13, 2016

Looks great, can we get a test case and an update the the docs on the manifest file format (build system.rst)?

@ankitspd
Copy link
Member Author

@ddunbar done


The default target to build can be specified by including an entry for the
empty string (`""`).
This section defines default target to build when manifest is loaded.
Copy link
Member

Choose a reason for hiding this comment

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

... defines the default ...

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@ddunbar
Copy link
Member

ddunbar commented Apr 13, 2016

LGTM, squash and merge away! Thanks!

@ankitspd
Copy link
Member Author

squashed, weirdly don't have merge access in this repo

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.

None yet

2 participants