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 15 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 comments must reference a GitHub issue for tracking. Format: TODO: and Issue URL: in next line.'
value: 'TODO:'
allowedPatterns: '^//\s*TODO:\s+.+'
anandwana001 marked this conversation as resolved.
Show resolved Hide resolved
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
12 changes: 9 additions & 3 deletions ground/src/main/java/com/google/android/ground/MainViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ constructor(

init {
viewModelScope.launch {
// TODO: Check auth status whenever fragments resumes
// 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 +88,9 @@ 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 +104,11 @@ 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,10 @@ 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 @@ -29,7 +29,7 @@ data class CaptureLocationTaskData(
val accuracy: Double?, // in metres
) : GeometryTaskData(location) {
override fun getDetailsText(): String {
// TODO: Move to strings.xml for i18n
// TODO(https://github.com/google/ground-android/issues/1733): Move to strings.xml for i18n
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,7 @@ 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(https://github.com/google/ground-android/issues/752): Move to strings.xml for i18n
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,7 @@ 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(https://github.com/google/ground-android/issues/1733): Move strings to view layer.
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,8 @@ 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(https://github.com/google/ground-android/issues/2910): Define these in data layer!
/** 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(https://github.com/google/ground-android/issues/1562): Preserve creation audit info for
// UPDATE mutations.
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(https://github.com/google/ground-android/issues/1562): Preserve creation audit info for
// UPDATE mutations.
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,7 +24,8 @@ 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
// TODO(https://github.com/google/ground-android/issues/950): Set IN_PROGRESS and FAILED statuses
// when necessary. On failure, set retry count and
// error and update to PENDING.
UNKNOWN(0, SyncStatus.UNKNOWN),

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(https://github.com/google/ground-android/issues/706): Apply pending local mutations before
// saving.
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(https://github.com/google/ground-android/issues/2743): Handle Kotlin Long
// (java.lang.Long) vs Java primitive long (long)
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(https://github.com/google/ground-android/issues/2031): Return SurveyListItem from
// getReadable(), only fetch required fields.
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(https://github.com/google/ground-android/issues/1748): Handle arrays, GeoPoint, int, and
// other types.
}
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,7 @@ private fun Message.hasValue(property: KProperty<*>): Boolean {
}

private fun MessageValue.toFirestoreValue(): FirestoreValue =
// TODO(#1748): Convert enums and other types.
// TODO(https://github.com/google/ground-android/issues/1748): Convert enums and other types.
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(https://github.com/google/ground-android/issues/2375): Switch to whereEqualTo(true) once
// legacy dev surveys deleted or migrated.
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(https://github.com/google/ground-android/issues/2392): Define field names on
// DocumentReference objects, not converters.
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(https://github.com/google/ground-android/issues/929): Set geometry once LOI has been
// updated to use our own model.
geometry = geometry,
submissionCount = submissionCount,
properties = properties,
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(https://github.com/google/ground-android/issues/2883): Apply mutations
// via repository layer rather than accessing data store directly.
remoteDataStore.applyMutations(mutations, user)
mutationRepository.finalizePendingMutationsForMediaUpload(mutations)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class WorkRequestBuilder {

companion object {
/** Backoff time should increase linearly. */
// TODO(#710): Check if it is possible to wake the worker as soon as the connection becomes
// TODO(https://github.com/google/ground-android/issues/710): Check if it is possible to wake
// the worker as soon as the connection becomes
// available, and if so switch to EXPONENTIAL backoff policy.
private val BACKOFF_POLICY = BackoffPolicy.LINEAR

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ constructor(
) {
/** Mirrors locations of interest in the specified survey from the remote db into the local db. */
suspend fun syncLocationsOfInterest(survey: Survey) {
// TODO(#2384): Allow survey organizers to make ad hoc LOIs visible to all data collectors.
// TODO(https://github.com/google/ground-android/issues/2384): Allow survey organizers to make
// ad hoc LOIs visible to all data collectors.
val ownerUserId = authenticationManager.getAuthenticatedUser().id
val lois =
with(remoteDataStore) { loadPredefinedLois(survey) + loadUserLois(survey, ownerUserId) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ constructor(
setOf(MEDIA_UPLOAD_PENDING, MEDIA_UPLOAD_IN_PROGRESS, MEDIA_UPLOAD_AWAITING_RETRY)
.contains(it.uploadStatus)
}
// TODO(https://github.com/google/ground-android/issues/2120):
// Return [MediaMutations] instead once introduced.
// TODO(https://github.com/google/ground-android/issues/2120): Return [MediaMutations]
// instead once introduced.
.mapNotNull { it.submissionMutation }

/**
Expand Down
Loading
Loading