-
-
Notifications
You must be signed in to change notification settings - Fork 45
Allow other module repositories #94
base: main
Are you sure you want to change the base?
Conversation
@@ -16,7 +16,7 @@ class RestService( | |||
|
|||
suspend fun getLatestRelease(repo: String) = withContext(Dispatchers.IO) { | |||
httpService.request<Release> { | |||
url("https://api.github.com/repos/vendetta-mod/$repo/releases/latest") | |||
url("https://api.github.com/repos/$repo/releases/latest") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is semantically correct. As you can see GitHub themselves refers the org/name segments as "repos" denoted by the segment before the repo.
import dev.beefers.vendetta.manager.domain.repository.RestRepository | ||
import dev.beefers.vendetta.manager.network.dto.Commit | ||
|
||
class CommitsPagingSource( | ||
private val repo: RestRepository | ||
private val repo: RestRepository, | ||
private val prefs: PreferenceManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if injecting preferences is clean, but seemed to be the intended approach.
@@ -115,6 +115,13 @@ class DeveloperSettings: Screen { | |||
onPrefChange = { prefs.debuggable = it } | |||
) | |||
|
|||
SettingsTextField( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The developer settings seemed the most suitable location.
@@ -17,7 +19,7 @@ class CommitsPagingSource( | |||
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, Commit> { | |||
val page = params.key ?: 0 | |||
|
|||
return when (val response = repo.getCommits("Vendetta", page)) { | |||
return when (val response = repo.getCommits(prefs.vendettaModuleRepo, page)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now displaying commits from Xposed module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commits of the module aren't what's relevant, the user doesn't care how the app is patched, they care about the mod injected afterwards as that is what they're using
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how to solve this other than adding another text field to specify where to get the commits from.
@@ -17,7 +17,7 @@ class DownloadVendettaStep( | |||
|
|||
override val nameRes = R.string.step_dl_vd | |||
|
|||
override val url: String = "https://github.com/vendetta-mod/VendettaXposed/releases/latest/download/app-release.apk" | |||
override val url: String = "https://github.com/${preferenceManager.vendettaModuleRepo}/releases/latest/download/app-release.apk" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the repo is configurable, the asset name is hardcoded, which means other module repo releases have to name their releases the same way.
release?.let { | ||
showUpdateDialog = it.tagName.toInt() > BuildConfig.VERSION_CODE | ||
} | ||
repo.getLatestRelease("VendettaXposed").ifSuccessful { | ||
repo.getLatestRelease(prefs.vendettaModuleRepo).ifSuccessful { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates are checked correctly for the configurable module repo.
About
This PR does two things.
It adds a developer setting to specify the module repository for Vendetta
It displays commits for the module repository instead of non-Xposed Vendetta. This makes sense because actual changes happen there
Why
Because Vendetta is now discontinued, this app can be repurposed in a certain way which prevents it to be useless when the module it injects is discontinued. Firstival is everyone that wants to create their own module required to fork this app. This means splitting the user base between the upstream version and all the other forkers. Second, if a forker discontinues their module, it means the death of their forked Vendetta Manager app.
With this PR both issues are solved. No one has to fork this app, this means users can get updates and don't have to hop to forked versions. Neither is the user forced to switch apps is a forker discontinues a module in the future. This PR ensures that the app can be useful for the future without turning useless just because the module is discontinued, by making the app modular and agnostic to the module it injects. This flexibility can be extended in the future to reduce the need to fork to a minimum.