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

Implement a build_config generator. #95

Closed
wants to merge 1 commit into from
Closed

Implement a build_config generator. #95

wants to merge 1 commit into from

Conversation

@nickpalmer
Copy link
Contributor

@nickpalmer nickpalmer commented Mar 4, 2014

Summary:
This change set adds a build_config generator that knows how to
generate the BuildConfig.java files generated by existing build tools.
This addresses issue: #3
and is based on the strategy proposed in the comment on that issue.

Test Plan:
There are already unit tests in place which verify all but the
contents of the generated file. To verify contents add a build_config
rule to a project and dig into buck-out to verify the contents
of the file are correct.

Summary:
This change set adds a build_config generator that knows how to
generate the BuildConfig.java files generated by existing build tools.
This addresses issue: #3
and is based on the strategy proposed in the comment on that issue.

Test Plan:
There are already unit tests in place which verify all but the
contents of the generated file. To verify contents add a build_config
rule to a project and dig into buck-out to verify the contents
of the file are correct.
@nickpalmer
Copy link
Contributor Author

@nickpalmer nickpalmer commented Mar 26, 2014

Note that I signed the CLA already. Let me know if there are any issues there.

@oconnor663
Copy link
Contributor

@oconnor663 oconnor663 commented Apr 11, 2014

Hey, sorry for the radio silence on this one. We're in the process of pulling it, and we'll post an update here when that's done. Thanks for the code! (And for making it a described rule, that really helps.)

natthu added a commit that referenced this pull request Apr 17, 2014
Summary:
Simplified the build steps a bit.
Use ProjectFilesystem in GenerateBuildConfigStep, and try-with-resources.
Use System.lineSeparator() for platform independent newlines.
Make field final.
Cleaned up unnecessary getter methods.
Simplified creating buildable/build rule in the unit test.
Fixed styling.

I will follow up with another diff that updates the soy docs.

Test Plan: buck test --all
@rowillia
Copy link
Contributor

@rowillia rowillia commented Apr 21, 2014

Pulled in 4c3b7f4

@rowillia rowillia closed this Apr 21, 2014
oconnor663 added a commit that referenced this pull request Apr 22, 2014
Summary:
Simplified the build steps a bit.
Use ProjectFilesystem in GenerateBuildConfigStep, and try-with-resources.
Use System.lineSeparator() for platform independent newlines.
Make field final.
Cleaned up unnecessary getter methods.
Simplified creating buildable/build rule in the unit test.
Fixed styling.

I will follow up with another diff that updates the soy docs.

Test Plan: buck test --all
@bolinfest
Copy link
Contributor

@bolinfest bolinfest commented Jul 9, 2014

Hi @nickpalmer, I ended up deleting build_config() in a recent commit and replacing it with android_build_config(). The API is different, but it makes it much more efficient to build debug and release versions of an android_binary() at the same time. You can see more information in the docs http://facebook.github.io/buck/rule/android_build_config.html and in the commit that introduced the new rule: d38375c.

@nickpalmer
Copy link
Contributor Author

@nickpalmer nickpalmer commented Jul 9, 2014

Nice optimization. I am glad Facebook gives you guys the time to focus on these issues! 👍

@nickpalmer nickpalmer deleted the nickpalmer:build-config branch Jul 9, 2014
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot facebook-github-bot commented Mar 23, 2016

@nickpalmer updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants