Split the compiler and runtime into separate artifacts. #323

Closed
wants to merge 8 commits into
from

Projects

None yet

3 participants

@serj-lotutovici
Contributor

Posible fix for #192
Didn't see #210 before it was too late (but the PR was not updated for a longer time, so I figured: why not!? :) )

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
butterknife/src/main/java/butterknife/ButterKnife.java
/**
* Field and method binding for Android views. Use this class to simplify finding views and
* attaching listeners by binding them with annotations.
- * <p>
+ * <p/>
@JakeWharton
JakeWharton Aug 1, 2015 Owner

Revert. This is invalid HTML3.2 which is what Javadoc uses. If you compile with JDK 8 it doclint will actually fail.

@JakeWharton
JakeWharton Aug 1, 2015 Owner

(all of them)

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
butterknife/src/main/java/butterknife/ButterKnife.java
* Finding views from your activity is as easy as:
* <pre><code>
* public class ExampleActivity extends Activity {
* {@literal @}Bind(R.id.title) EditText titleView;
* {@literal @}Bind(R.id.subtitle) EditText subtitleView;
- *
+ * <p/>
@JakeWharton
JakeWharton Aug 1, 2015 Owner

This isn't between a paragraph.

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
.../src/main/java/butterknife/internal/InternalKeys.java
@@ -0,0 +1,15 @@
+package butterknife.internal;
+
+/**
+ * Contains shared constants between the annotation processor and {@link butterknife.ButterKnife}
+ */
+public final class InternalKeys {
+ public static final String BINDING_CLASS_SUFFIX = "$$ViewBinder";
+ public static final String ANDROID_PREFIX = "android.";
+ public static final String JAVA_PREFIX = "java.";
+
+ /** Prevent class from instantiation (aka being paranoid) */
@JakeWharton
JakeWharton Aug 1, 2015 Owner

Delete comment.

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
.../src/main/java/butterknife/internal/InternalKeys.java
@@ -0,0 +1,15 @@
+package butterknife.internal;
+
+/**
+ * Contains shared constants between the annotation processor and {@link butterknife.ButterKnife}
+ */
+public final class InternalKeys {
+ public static final String BINDING_CLASS_SUFFIX = "$$ViewBinder";
+ public static final String ANDROID_PREFIX = "android.";
+ public static final String JAVA_PREFIX = "java.";
+
+ /** Prevent class from instantiation (aka being paranoid) */
+ private InternalKeys() {
+ throw new AssertionError("InternalKeys is a constant container. Should not be instantiated");
@JakeWharton
JakeWharton Aug 1, 2015 Owner

Typical message here is just "No instances."

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
butterknife/pom.xml
@@ -60,7 +54,6 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<configuration>
- <excludePackageNames>butterknife.internal:butterknife.internal.*</excludePackageNames>
@JakeWharton
JakeWharton Aug 1, 2015 Owner

This is still needed to ensure InternalKeys doesn't show up in Javadoc.

@JakeWharton JakeWharton commented on an outdated diff Aug 1, 2015
butterknife/pom.xml
@@ -52,7 +47,6 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
- <compilerArgument>-proc:none</compilerArgument>
</configuration>
</plugin>
@JakeWharton
JakeWharton Aug 1, 2015 Owner

You can delete this whole <plugin>

@JakeWharton JakeWharton commented on the diff Aug 1, 2015
butterknife-compiler/pom.xml
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.easytesting</groupId>
+ <artifactId>fest-assert-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.squareup</groupId>
+ <artifactId>fest-android</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.robolectric</groupId>
@JakeWharton
JakeWharton Aug 1, 2015 Owner

This dependency can probably be removed

@serj-lotutovici
serj-lotutovici Aug 1, 2015 Contributor

Unfortunately not. AllTheThingsTest and OnPageChangeTest indirectly require it. Removing it will result in test failure do compilation error after the ButterKnifeProcessor execution.

@JakeWharton
Owner

This looks fantastic! I have only a few minor comments, then let's get this merged!

Serj Lotutovici added some commits Aug 1, 2015
@serj-lotutovici
Contributor

@JakeWharton fixed issues, based on your comments. By chance do you have any "code style" profile file for intellij idea, that I can use? It seams that my ide settings profile defers from yours, which can cause same formatting issue in the future. Thx in advance :)

@JakeWharton
Owner

Squashed and merged.

@JakeWharton JakeWharton closed this Aug 2, 2015
@JakeWharton
Owner

Thanks for this!

@serj-lotutovici serj-lotutovici deleted the serj-lotutovici:sl/split_compiler_and_runtime branch Aug 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment