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

Ref: Bump AGP to 4.2.x and fix linter issues #1137

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion buildSrc/src/main/java/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ object Config {
val springKotlinCompatibleLanguageVersion = "1.3"

object BuildPlugins {
val androidGradle = "com.android.tools.build:gradle:4.1.1"
val androidGradle = "com.android.tools.build:gradle:4.2.0-beta02"
val kotlinGradlePlugin = "gradle-plugin"
val buildConfig = "com.github.gmazzo.buildconfig"
val buildConfigVersion = "2.0.2"
Expand Down
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-6.6.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7.1-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 2 additions & 0 deletions sentry-android-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ android {

// We run a full lint analysis as build part in CI, so skip vital checks for assemble tasks.
isCheckReleaseBuilds = false

disable.add("UnknownNullness")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philipphofmann help me to fix those
so no manoelly linter :)

Copy link
Member

Choose a reason for hiding this comment

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

LOL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

basic all public getters and setters would need either @NotNull or @nullable, overwhelming hehe, I'd guess around 700

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just added this disable rule to make CI happier and focus on the other lint issues.

Copy link
Member

Choose a reason for hiding this comment

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

image

}

// needed because of Kotlin 1.4.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ final class AndroidLogger implements ILogger {

private static final String tag = "Sentry";

@SuppressWarnings("AnnotateFormatMethod")
@SuppressWarnings({"AnnotateFormatMethod", "LogConditional"})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we check if logger is enabled by ourselves, we don't need to check the BuildConfig.DEBUG
Sentry logger != Android logger

@Override
public void log(SentryLevel level, String message, Object... args) {
Log.println(toLogcatLevel(level), tag, String.format(message, args));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public abstract class EnvelopeFileObserverIntegration implements Integration, Cl
private @Nullable EnvelopeFileObserver observer;
private @Nullable ILogger logger;

@SuppressWarnings("SyntheticAccessor")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are a bunch of SyntheticAccessor errors for private static final classes, not sure if they make sense or are false positive, I don't see how could it be problematic

public static @NotNull EnvelopeFileObserverIntegration getOutboxFileObserver() {
return new OutboxEnvelopeFileObserverIntegration();
}
Expand Down Expand Up @@ -67,6 +68,7 @@ public void close() {
@TestOnly
abstract @Nullable String getPath(final @NotNull SentryOptions options);

@SuppressWarnings("SyntheticAccessor")
private static final class OutboxEnvelopeFileObserverIntegration
extends EnvelopeFileObserverIntegration {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ private void scheduleEndSession() {
@Override
public void run() {
addSessionBreadcrumb("end");
hub.endSession();
runningSession.set(false);
getHub().endSession();
isRunningSession().set(false);
}
};

Expand All @@ -127,7 +127,7 @@ private void addAppBreadcrumb(final @NotNull String state) {
}
}

private void addSessionBreadcrumb(final @NotNull String state) {
void addSessionBreadcrumb(final @NotNull String state) {
final Breadcrumb breadcrumb = new Breadcrumb();
breadcrumb.setType("session");
breadcrumb.setData("state", state);
Expand All @@ -136,7 +136,6 @@ private void addSessionBreadcrumb(final @NotNull String state) {
hub.addBreadcrumb(breadcrumb);
}

@TestOnly
@NotNull
AtomicBoolean isRunningSession() {
return runningSession;
Expand All @@ -147,4 +146,9 @@ AtomicBoolean isRunningSession() {
TimerTask getTimerTask() {
return timerTask;
}

@NotNull
IHub getHub() {
return hub;
}
}
2 changes: 2 additions & 0 deletions sentry-android-ndk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ android {

// We run a full lint analysis as build part in CI, so skip vital checks for assemble tasks.
isCheckReleaseBuilds = false

disable.add("UnknownNullness")
}

nativeBundleExport {
Expand Down
2 changes: 2 additions & 0 deletions sentry-android-timber/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ android {

// We run a full lint analysis as build part in CI, so skip vital checks for assemble tasks.
isCheckReleaseBuilds = false

disable.add("UnknownNullness")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class SentrySpanAdviceTest {

@Test
fun `when method is annotated with @SentrySpan and there is no active transaction, span is not created and method is executed`() {
val scope = Scope(SentryOptions())
Scope(SentryOptions())
whenever(hub.span).thenReturn(null)
val result = sampleService.methodWithSpanDescriptionSet()
assertEquals(1, result)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import io.sentry.SentryOptions
/**
* Verifies is [SentryEnvelope] contains first event matching a predicate.
*/
inline fun checkEvent(noinline predicate: (SentryEvent) -> Unit): SentryEnvelope {
fun checkEvent(predicate: (SentryEvent) -> Unit): SentryEnvelope {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no reason to be inlined, first because has no effect for this usecase, 2nd because its a testing class

val options = SentryOptions().apply {
setSerializer(GsonSerializer(NoOpLogger.getInstance(), envelopeReader))
}
Expand Down
1 change: 1 addition & 0 deletions sentry/src/main/java/io/sentry/CircularFifoQueue.java
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ private int decrement(int index) {
*
* @return an iterator over this queue's elements
*/
@SuppressWarnings("SyntheticAccessor")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to not touch this class that is borrowed from Apache package

@Override
public Iterator<E> iterator() {
return new Iterator<E>() {
Expand Down
8 changes: 6 additions & 2 deletions sentry/src/main/java/io/sentry/DateUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ public final class DateUtils {
// if UTC is not found, it fallback to "GMT" which is UTC equivalent
private static final @NotNull TimeZone UTC_TIMEZONE = TimeZone.getTimeZone(UTC);

static @NotNull TimeZone getUtCTimezone() {
return UTC_TIMEZONE;
}

private static final @NotNull ThreadLocal<SimpleDateFormat> SDF_ISO_FORMAT_WITH_MILLIS_UTC =
new ThreadLocal<SimpleDateFormat>() {
@Override
protected SimpleDateFormat initialValue() {
final SimpleDateFormat simpleDateFormat =
new SimpleDateFormat(ISO_FORMAT_WITH_MILLIS, Locale.ROOT);
simpleDateFormat.setTimeZone(UTC_TIMEZONE);
simpleDateFormat.setTimeZone(getUtCTimezone());
return simpleDateFormat;
}
};
Expand All @@ -37,7 +41,7 @@ protected SimpleDateFormat initialValue() {
@Override
protected SimpleDateFormat initialValue() {
final SimpleDateFormat simpleDateFormat = new SimpleDateFormat(ISO_FORMAT, Locale.ROOT);
simpleDateFormat.setTimeZone(UTC_TIMEZONE);
simpleDateFormat.setTimeZone(getUtCTimezone());
return simpleDateFormat;
}
};
Expand Down
Loading