Skip to content

Commit

Permalink
Automate the TODO format check and fix older format (#2904)
Browse files Browse the repository at this point in the history
  • Loading branch information
anandwana001 authored Dec 19, 2024
1 parent 543795e commit f1a2ca8
Show file tree
Hide file tree
Showing 83 changed files with 166 additions and 123 deletions.
6 changes: 3 additions & 3 deletions config/detekt/detekt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,9 @@ style:
value: 'FIXME:'
- reason: 'Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.'
value: 'STOPSHIP:'
# - reason: 'Forbidden TODO todo marker in comment, please do the changes.'
# value: 'TODO:'
allowedPatterns: ''
- reason: 'TODO comment format should be: // TODO: $description'
value: '// TODO:'
allowedPatterns: '^// TODO: .+$'
ForbiddenImport:
active: false
imports: [ ]
Expand Down
6 changes: 4 additions & 2 deletions ground/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ android {
minSdkVersion rootProject.androidMinSdk
targetSdkVersion rootProject.androidTargetSdk

// TODO(https://github.com/google/ground-android/pull/985): Calculate version code manually
// TODO: Calculate version code manually
// Issue URL: https://github.com/google/ground-android/pull/985
versionCode gitVersioner.versionCode
versionName gitVersioner.versionName + " " + getCommitSha1()
testInstrumentationRunner "com.google.android.ground.CustomTestRunner"
Expand Down Expand Up @@ -358,7 +359,8 @@ dependencies {

implementation "com.google.guava:guava:33.0.0-android"

// TODO(#1748): Move protos into shared module and set correct path here.
// TODO: Move protos into shared module and set correct path here.
// Issue URL: https://github.com/google/ground-android/issues/1748
api("com.google.protobuf:protobuf-kotlin-lite:4.26.1")

// Pulls protodefs from the specified commit in the ground-platform repo.
Expand Down
4 changes: 3 additions & 1 deletion ground/src/main/java/com/google/android/ground/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ object Config {
*/
const val CLUSTERING_ZOOM_THRESHOLD = 14f

// TODO(#1730): Make sub-paths configurable and stop hardcoding here.
// TODO: Make sub-paths configurable and
// stop hardcoding here.
// Issue URL: https://github.com/google/ground-android/issues/1730
const val DEFAULT_MOG_TILE_LOCATION = "/offline-imagery/default"
private const val DEFAULT_MOG_MIN_ZOOM = 8
private const val DEFAULT_MOG_MAX_ZOOM = 14
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ class MainActivity : AbstractActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
// Make sure this is before calling super.onCreate()
setTheme(R.style.AppTheme)
// TODO(#620): Remove this to enable dark theme.
// TODO: Remove this to enable dark theme.
// Issue URL: https://github.com/google/ground-android/issues/620
AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO)
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -135,7 +136,9 @@ class MainActivity : AbstractActivity() {
var showDialog by remember { mutableStateOf(true) }
if (showDialog) {
PermissionDeniedDialog(
// TODO(#2402): Read url from Firestore config/properties/signUpUrl
// TODO: Read url from
// Firestore config/properties/signUpUrl
// Issue URL: https://github.com/google/ground-android/issues/2402
BuildConfig.SIGNUP_FORM_LINK,
onSignOut = {
showDialog = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ constructor(
init {
viewModelScope.launch {
// TODO: Check auth status whenever fragments resumes
// Issue URL: https://github.com/google/ground-android/issues/2624
authenticationManager.signInState.collect {
_navigationRequests.emit(onSignInStateChange(it))
}
Expand All @@ -86,7 +87,8 @@ constructor(
Timber.d("User cancelled sign in")
MainUiState.OnUserSignedOut
} else {
// TODO(#1808): Display some error dialog to the user with a helpful user-readable message.
// TODO: Display some error dialog to the user with a helpful user-readable message.
// Issue URL: https://github.com/google/ground-android/issues/1808
onUserSignedOut()
}
}
Expand All @@ -100,9 +102,10 @@ constructor(
surveyRepository.clearActiveSurvey()
userRepository.clearUserPreferences()

// TODO(#1691): Once multi-user login is supported, avoid clearing local db data. This is
// TODO: Once multi-user login is supported, avoid clearing local db data. This is
// currently being done to prevent one user's data to be submitted as another user after
// re-login.
// Issue URL: https://github.com/google/ground-android/issues/1691
viewModelScope.launch { withContext(ioDispatcher) { localDatabase.clearAllTables() } }
return MainUiState.OnUserSignedOut
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ data class LinearRing(val coordinates: List<Coordinates>) : Geometry {
override fun isEmpty() = coordinates.isEmpty()

override fun validate() {
// TODO(#1647): Check for vertices count > 3
// TODO: Check for vertices count > 3
// Issue URL: https://github.com/google/ground-android/issues/1647
if (coordinates.isEmpty()) {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ data class Job(
val tasksSorted: List<Task>
get() = tasks.values.sortedBy { it.index }

// TODO(#2216): Consider using nulls to indicate absence of value here instead of throwing
// an exception.
// TODO: Consider using nulls to indicate absence of value here instead of throwing an exception.
// Issue URL: https://github.com/google/ground-android/issues/2216
fun getTask(id: String): Task = tasks[id] ?: throw TaskNotFoundException(id)

/** Job must contain at-most 1 `AddLoiTask`. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ data class LocationOfInterest(
* database.
*/
// TODO: Remove this test-only method
// Issue URL: https://github.com/google/ground-android/issues/2903
fun toMutation(type: Mutation.Type, userId: String): LocationOfInterestMutation =
LocationOfInterestMutation(
jobId = job.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ data class CaptureLocationTaskData(
) : GeometryTaskData(location) {
override fun getDetailsText(): String {
// TODO: Move to strings.xml for i18n
// Issue URL: https://github.com/google/ground-android/issues/1733
val df = DecimalFormat("#.##")
df.roundingMode = RoundingMode.DOWN
val coordinatesString = location.coordinates.toDmsFormat()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import java.text.DecimalFormat
/** User-provided response to a "drop a pin" data collection [Task]. */
data class DropPinTaskData(val location: Point) : GeometryTaskData(location) {
override fun getDetailsText(): String {
// TODO(#752): Move to strings.xml for i18n
// TODO: Move to strings.xml for i18n
// Issue URL: https://github.com/google/ground-android/issues/752
val df = DecimalFormat("#.##")
df.roundingMode = RoundingMode.DOWN
return location.coordinates.toDmsFormat()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import com.google.android.ground.model.geometry.Polygon
/** A user-provided response to a geometry-based task ("drop a pin" or "draw an area"). */
abstract class GeometryTaskData(val geometry: Geometry) : TaskData {

// TODO(#1733): Move strings to view layer.
// TODO: Move strings to view layer.
// Issue URL: https://github.com/google/ground-android/issues/1733
override fun getDetailsText(): String =
when (geometry) {
is Point -> "Point data"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class MultipleChoiceTaskData(
suffix = MultipleChoiceTaskViewModel.OTHER_SUFFIX,
) ?: ""

// TODO: Make these inner classes non-static and access Task directly.
override fun getDetailsText(): String =
selectedOptionIds
.mapNotNull { multipleChoice?.getOptionById(it) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ package com.google.android.ground.model.submission
* @property data A map from task id to values. This map is mutable and therefore should never be
* exposed outside this class.
*/
// TODO: Merge into Submission?
data class SubmissionData(private val data: Map<String, TaskData?> = mapOf()) {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ constructor(
val condition: Condition? = null,
) {

/**
* Task type names as they appear in the remote db, but in uppercase. DO NOT RENAME! TODO: Define
* these in data layer!
*/
// TODO: Define these in data layer!
// Issue URL: https://github.com/google/ground-android/issues/2910
// Task type names as they appear in the local db, but in uppercase. DO NOT RENAME!
enum class Type {
UNKNOWN,
TEXT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ fun LocationOfInterestMutation.toLocalDataStoreObject(user: User): LocationOfInt
surveyId = surveyId,
jobId = jobId,
deletionState = EntityDeletionState.DEFAULT,
// TODO(#1562): Preserve creation audit info for UPDATE mutations.
// TODO: Preserve creation audit info for UPDATE mutations.
// Issue URL: https://github.com/google/ground-android/issues/1562
created = auditInfo,
lastModified = auditInfo,
geometry = geometry?.toLocalDataStoreObject(),
Expand Down Expand Up @@ -321,7 +322,8 @@ fun SubmissionMutation.toLocalDataStoreObject(created: AuditInfo): SubmissionEnt
locationOfInterestId = this.locationOfInterestId,
deletionState = EntityDeletionState.DEFAULT,
data = SubmissionDataConverter.toString(SubmissionData().copyWithDeltas(this.deltas)),
// TODO(#1562): Preserve creation audit info for UPDATE mutations.
// TODO: Preserve creation audit info for UPDATE mutations.
// Issue URL: https://github.com/google/ground-android/issues/1562
created = auditInfo,
lastModified = auditInfo,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ internal object ValueJsonConverter {
private fun toJsonArray(response: MultipleChoiceTaskData): JSONArray =
JSONArray().apply { response.selectedOptionIds.forEach { this.put(it) } }

// TODO: Replace with proto conversion logic if this is still necessary
fun toResponse(task: Task, obj: Any): TaskData? {
if (JSONObject.NULL === obj) {
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ data class UserEntity(
@ColumnInfo(name = "id") @PrimaryKey val id: String,
@ColumnInfo(name = "email") val email: String,
@ColumnInfo(name = "display_name") val displayName: String,
// TODO(https://github.com/google/ground-android/issues/964): Save to remote db
// TODO: Save to remote db
// Issue URL: https://github.com/google/ground-android/issues/964
@ColumnInfo(name = "photo_url") val photoUrl: String?,
)
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import com.google.android.ground.persistence.local.room.IntEnum.Companion.toInt
/** Mutually exclusive mutations states. */
enum class MutationEntitySyncStatus(private val intValue: Int, private val enumValue: SyncStatus) :
IntEnum {
// TODO(#950): Set IN_PROGRESS and FAILED statuses when necessary. On failure, set retry count and
// error and update to PENDING.
// TODO: Set IN_PROGRESS and FAILED statuses
// when necessary. On failure, set retry count and error and update to PENDING.
// Issue URL: https://github.com/google/ground-android/issues/950
UNKNOWN(0, SyncStatus.UNKNOWN),

/** Pending includes failed sync attempts pending retry. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class RoomLocationOfInterestStore @Inject internal constructor() : LocalLocation
): LocationOfInterest? =
locationOfInterestDao.findById(locationOfInterestId)?.toModelObject(survey)

// TODO(#706): Apply pending local mutations before saving.
// TODO: Apply pending local mutations before saving.
// Issue URL: https://github.com/google/ground-android/issues/706
override suspend fun merge(model: LocationOfInterest) {
locationOfInterestDao.insertOrUpdate(model.toLocalDataStoreObject())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ open class DataStoreException(message: String?) : RuntimeException(message) {
@JvmStatic
@Throws(DataStoreException::class)
fun <T : Any> checkType(expectedType: Class<*>, obj: T): T {
// TODO(#2743) - Handle Kotlin Long (java.lang.Long) vs Java primitive long (long)
// TODO: Handle Kotlin Long (java.lang.Long) vs Java primitive long (long)
// Issue URL: https://github.com/google/ground-android/issues/2743
if (obj.javaClass == java.lang.Long::class.java && expectedType == Long::class.java) {
return obj
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.tasks.await
import timber.log.Timber

// TODO: Add column to Submission table for storing uploaded media urls
// TODO: Synced to remote db as well
@Singleton
class FirebaseStorageManager @Inject constructor() : RemoteStorageManager {

Expand Down Expand Up @@ -61,7 +59,6 @@ class FirebaseStorageManager @Inject constructor() : RemoteStorageManager {
*
* user-media/surveys/{survey_id}/submissions/{field_id-uuid.jpg}
*/
// TODO: Refactor this into MediaStorageRepository.
fun getRemoteMediaPath(surveyId: String, filename: String): String =
StringJoiner(File.separator)
.add(MEDIA_ROOT_DIR)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ internal constructor(
override fun getSurveyList(user: User): Flow<List<SurveyListItem>> = flow {
emitAll(
db().surveys().getReadable(user).map { list ->
// TODO(#2031): Return SurveyListItem from getReadable(), only fetch required fields.
// TODO: Return SurveyListItem from getReadable(), only fetch required fields.
// Issue URL: https://github.com/google/ground-android/issues/2031
list.map { it.toListItem(false) }
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,5 @@ open class FluentFirestore protected constructor(private val db: FirebaseFiresto

protected fun db(): FirebaseFirestore = db

// TODO: Wrap in fluent version of WriteBatch.
fun batch(): WriteBatch = db.batch()
}
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,6 @@ private fun FirestoreValue.toMessageValue(targetType: KClass<*>): MessageValue =
?: throw IllegalArgumentException("Unrecognized enum number $this")
} else {
throw UnsupportedOperationException("Unsupported message field type $targetType")
// TODO(#1748): Handle arrays, GeoPoint, int, and other types.
// TODO: Handle arrays, GeoPoint, int, and other types.
// Issue URL: https://github.com/google/ground-android/issues/1748
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import com.google.protobuf.timestamp
import java.util.Date
import kotlinx.collections.immutable.toImmutableMap

// TODO: Add test coverage
fun SubmissionMutation.createSubmissionMessage(user: User) = submission {
assert(userId == user.id) { "UserId doesn't match: expected $userId, found ${user.id}" }

Expand Down Expand Up @@ -127,7 +126,6 @@ fun LocationOfInterestMutation.createLoiMessage(user: User) = locationOfInterest
}

private fun toTaskData(id: String, newTaskData: TaskData) = taskData {
// TODO: What should be the ID?
taskId = id

when (newTaskData) {
Expand All @@ -147,7 +145,6 @@ private fun toTaskData(id: String, newTaskData: TaskData) = taskData {
newTaskData.altitude?.let { altitude = it }
newTaskData.accuracy?.let { accuracy = it }
coordinates = newTaskData.location.coordinates.toMessage()
// TODO: Add timestamp
}
is GeometryTaskData -> drawGeometryResult = drawGeometryResult {
geometry = newTaskData.geometry.toMessage()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ private fun Message.hasValue(property: KProperty<*>): Boolean {
}

private fun MessageValue.toFirestoreValue(): FirestoreValue =
// TODO(#1748): Convert enums and other types.
// TODO: Convert enums and other types.
// Issue URL: https://github.com/google/ground-android/issues/1748
when (this) {
is List<*> -> map { it?.toFirestoreValue() }
is Message -> toFirestoreMap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class LoiCollectionReference internal constructor(ref: CollectionReference) :
/** Retrieves all "predefined" LOIs in the specified survey. Main-safe. */
suspend fun fetchPredefined(survey: Survey): List<LocationOfInterest> =
// Use !=false rather than ==true to not break legacy dev surveys.
// TODO(#2375): Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated.
// TODO: Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated.
// Issue URL: https://github.com/google/ground-android/issues/2375
fetchLois(
survey,
reference().whereEqualTo(SOURCE_FIELD, LocationOfInterestProto.Source.IMPORTED.number),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import com.google.android.ground.proto.LocationOfInterest as LocationOfInterestP
import com.google.android.ground.proto.LocationOfInterest.Source
import com.google.firebase.firestore.DocumentSnapshot

// TODO: Add tests.
/** Converts between Firestore documents and [LocationOfInterest] instances. */
object LoiConverter {
// TODO(#2392): Define field names on DocumentReference objects, not converters.
// TODO: Define field names on DocumentReference objects, not converters.
// Issue URL: https://github.com/google/ground-android/issues/2375
const val GEOMETRY_TYPE = "type"
const val POLYGON_TYPE = "Polygon"

Expand Down Expand Up @@ -71,7 +71,8 @@ object LoiConverter {
job = job,
created = created,
lastModified = lastModified,
// TODO(#929): Set geometry once LOI has been updated to use our own model.
// TODO: Set geometry once LOI has been updated to use our own model.
// Issue URL: https://github.com/google/ground-android/issues/929
geometry = geometry,
submissionCount = submissionCount,
properties = properties,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class SurveyDocumentReference internal constructor(ref: DocumentReference) :
suspend fun get(): Survey? {
try {
val surveyDoc = reference().get().await()
// TODO(https://github.com/google/ground-android/issues/2864): Move jobs fetch to outside this
// DocumentReference class.
// TODO: Move jobs fetch to outside this DocumentReference class.
// Issue URL: https://github.com/google/ground-android/issues/2864
val jobs = jobs().get()
return SurveyConverter.toSurvey(surveyDoc, jobs)
} catch (e: CancellationException) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ constructor(
try {
val user = userRepository.getAuthenticatedUser()
mutationRepository.markAsInProgress(mutations)
// TODO(https://github.com/google/ground-android/issues/2883):
// Apply mutations via repository layer rather than accessing data store directly.
// TODO: Apply mutations via repository layer rather than accessing data store directly.
// Issue URL: https://github.com/google/ground-android/issues/2883
remoteDataStore.applyMutations(mutations, user)
mutationRepository.finalizePendingMutationsForMediaUpload(mutations)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ constructor(
} else {
mutationRepository.markAsFailedMediaUpload(
listOf(mutation),
// TODO(https://github.com/google/ground-android/issues/2120): Replace this workaround with
// update of specific [MediaMutation], aggregate to [UploadQueueEntry] for display in UI.
// TODO: Replace this workaround with update of specific [MediaMutation],
// aggregate to [UploadQueueEntry] for display in UI.
// Issue URL: https://github.com/google/ground-android/issues/2120
results.firstNotNullOfOrNull { it.exceptionOrNull() } ?: UnknownError(),
)
return false
Expand Down
Loading

0 comments on commit f1a2ca8

Please sign in to comment.