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

Automate the TODO format check and fix older format #2904

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
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
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: '^//\s*TODO:\s+.+$'
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
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -63,7 +63,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 @@ -122,7 +123,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
Loading