Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Add kotlin rules support to buck project #1200

Closed
wants to merge 2 commits into from

Conversation

kageiit
Copy link
Contributor

@kageiit kageiit commented Feb 22, 2017

No description provided.

@kageiit
Copy link
Contributor Author

kageiit commented Feb 22, 2017

cc @dsyang

Copy link
Contributor

@dsyang dsyang left a comment

Choose a reason for hiding this comment

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

lgtm

ModuleBuildContext context) {
addDepsAndSources(
target,
false /* wantsPackagePrefix */,
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: What does this do?

Copy link
Contributor Author

@kageiit kageiit Feb 22, 2017

Choose a reason for hiding this comment

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

I just copied it from the groovy rule :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More info here:

public boolean getWantsPackagePrefix() {

Copy link
Contributor

Choose a reason for hiding this comment

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

this option is there mostly because fb source layout is weird and doesn't match Intellij convention. At the same time, given the similarity between kotlin and java, it might not be a bad idea to have it. Give it a shot, if it works with this set to true maybe just leave it in?

Copy link
Contributor

@dsyang dsyang left a comment

Choose a reason for hiding this comment

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

Actually, looks like there's some failing tests.

@kageiit
Copy link
Contributor Author

kageiit commented Feb 22, 2017

Ill fix the tests and re-upload. ant tests passed for me. Need to look at the buck build path

@facebook-github-bot
Copy link
Contributor

@kageiit updated the pull request - view changes

@facebook-github-bot
Copy link
Contributor

@dsyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@dsyang
Copy link
Contributor

dsyang commented Feb 24, 2017

@kageiit can you rebase again? Some internal tests are failing and I think it's b/c of a merge conflict

@kageiit kageiit force-pushed the kotlin_project_support branch from db4d3dd to 92872a6 Compare February 24, 2017 03:18
@kageiit
Copy link
Contributor Author

kageiit commented Feb 24, 2017

@dsyang rebased on latest and force pushed

@facebook-github-bot
Copy link
Contributor

@kageiit updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@dsyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@kageiit
Copy link
Contributor Author

kageiit commented Mar 1, 2017

@dsyang any update on the import?

@dsyang
Copy link
Contributor

dsyang commented Mar 2, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants