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 1 commit
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: 'TODO comments must reference a GitHub issue for tracking. Format: TODO(https://github.com/google/ground-android/issues/IssueNumber).'
value: '(TODO|TODO\(|TODO\(#)'
allowedPatterns: '^TODO\(https://github\.com/google/ground-android/issues/\d+\): .+'
- 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(https://github.com/google/ground-android/issues/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
5 changes: 3 additions & 2 deletions ground/src/main/java/com/google/android/ground/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ object Config {
*/
const val CLUSTERING_ZOOM_THRESHOLD = 14f

// TODO(https://github.com/google/ground-android/issues/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(https://github.com/google/ground-android/issues/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,8 +123,9 @@ class MainActivity : AbstractActivity() {
var showDialog by remember { mutableStateOf(true) }
if (showDialog) {
PermissionDeniedDialog(
// TODO(https://github.com/google/ground-android/issues/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
15 changes: 9 additions & 6 deletions ground/src/main/java/com/google/android/ground/MainViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ constructor(

init {
viewModelScope.launch {
// TODO(https://github.com/google/ground-android/issues/2624): 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 @@ -87,8 +88,9 @@ constructor(
Timber.d("User cancelled sign in")
MainUiState.OnUserSignedOut
} else {
// TODO(https://github.com/google/ground-android/issues/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 @@ -102,10 +104,11 @@ constructor(
surveyRepository.clearActiveSurvey()
userRepository.clearUserPreferences()

// TODO(https://github.com/google/ground-android/issues/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(https://github.com/google/ground-android/issues/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,9 +41,10 @@ data class Job(
val tasksSorted: List<Task>
get() = tasks.values.sortedBy { it.index }

// TODO(https://github.com/google/ground-android/issues/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 @@ -58,7 +58,8 @@ data class LocationOfInterest(
* Converts this LOI to a mutation that can be used to update this LOI in the remote and local
* database.
*/
// TODO(https://github.com/google/ground-android/issues/2903): Remove this test-only method
// 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
Loading