[Reminder] Add reminder skill with Tasks.org integration#391
[Reminder] Add reminder skill with Tasks.org integration#391keithvassallomt wants to merge 2 commits intoStypox:masterfrom
Conversation
Resolve conflicts by keeping both reminder skill and upstream's notify/flashlight additions in SkillHandler and full_description.
|
Just bumping this :) If any further changes are required I'm more than happy to accommodate! |
Stypox
left a comment
Stypox
left a comment
There was a problem hiding this comment.
Hello and thank you for the pull request. If I understand correctly, most of the code is generated by AI, including the PR body. I don't feel too happy with merging code I didn't write, but for which I still need to take full responsibility because no other human wrote/reviewed it properly. I still gave a very quick review though.
I did not read the PR body as that is most likely AI generated too, and as such is mostly useless. Please go through the changes in this PR, review them using your own knowledge, and if needed make changes to make the code better, e.g. use/put helper functions in util files that already exist. Finally, write a pull request body on your own without using an LLM.
I am sorry if I am coming off as harsh here, but you have to understand that the maintainer duties and responsibilities have remained the same as before AI existed. But now the creation of pull requests that are seemingly legitimate has become orders of magnitude easier, and maintainers still have to put in the same technical and emotional efforts into each of them.
One last thing: this skill's sentences are probably going to conflict with #381 so we will have to figure something out.
| // Play Store not available, try browser | ||
| val webIntent = Intent( | ||
| Intent.ACTION_VIEW, | ||
| Uri.parse("https://play.google.com/store/apps/details?id=org.tasks") |
There was a problem hiding this comment.
Please use https://f-droid.org/packages/org.tasks/ instead
| Headline(text = getSpeechOutput(ctx)) | ||
| Spacer(modifier = Modifier.height(8.dp)) | ||
| TextButton(onClick = { | ||
| val intent = Intent( |
| @@ -246,4 +246,19 @@ | |||
| <string name="skill_name_translation">Překlad</string> | |||
There was a problem hiding this comment.
Delete all of the strings translations except those in english, which is the source language. Translations are made through Weblate.
| @@ -0,0 +1,5 @@ | |||
| create: | |||
There was a problem hiding this comment.
Delete all of the sentence files except for languages you know. LLMs are very bad at writing these files.
| } | ||
|
|
||
| companion object { | ||
| private const val TASKS_ORG_PACKAGE = "org.tasks" |
There was a problem hiding this comment.
Mmmh, isn't there a built in Android API to achieve something like this? Could you do some research (you, not AI)?
| rememberVectorPainter(Icons.Default.Notifications) | ||
|
|
||
| override fun isAvailable(ctx: SkillContext): Boolean { | ||
| return Sentences.Reminder[ctx.sentencesLanguage] != null |
There was a problem hiding this comment.
Also make this unavailable if the Tasks.org app is not installed. Move the check over from the other file.
| var taskTitle = rawTask | ||
| var dateTime: LocalDateTime? = null | ||
|
|
||
| if (parserFormatter != null) { |
There was a problem hiding this comment.
Make use of the new date time matching introduced in d701d86
| } | ||
|
|
||
| if (rawTask.isBlank()) { | ||
| return ReminderOutput.Created(title = "", dateTime = null) |
There was a problem hiding this comment.
Create another class specifically for this
|
Thank you for your response and for the time you've taken to review this PR. Of course, all of the points you've raised are 100% correct, except one - I really did write the PR body myself - guess I just sound a lot like AI 😅 I could make all the changes you have requested, but honestly I don't want to waste any more of your time. Your opinion (which is, once again, entirely valid) is that a PR with AI-generated code is only "seemingly legitimate", in other words it is illegitimate. Your position is entirely understandable from a maintainer's standpoint. I've decided not to move forward with this and another PR I was working on. I apologise for the noise. |
2 participants
Summary
Adds a new reminder skill that creates tasks in Tasks.org via voice commands. The skill uses the twofortyfouram Locale plugin broadcast protocol to communicate with Tasks.org.
Details
New files
ReminderSkill.kt— Core skill logic usingStandardRecognizerSkill, broadcasts task to Tasks.orgReminderOutput.kt— Compose UI output confirming task creationReminderInfo.kt— Skill registration with settings (priority, voice input saving)SkillSettingsReminderSerializer.kt— Proto DataStore serializer for settingsskill_settings_reminder.proto— Protobuf schema for reminder settingsreminder.ymlsentence pattern files (one per language)strings.xmlupdates with 15 translated strings eachModified files
SkillHandler.kt— RegisteredReminderInfoin the skill listskill_definitions.yml— Addedreminderskill definitionREADME.md— Added reminder to the skills listfull_description.txt— Added reminder to the F-Droid/Play Store descriptionTest plan
./gradlew assembleDebugbuilds successfullyNote
The non-English sentence patterns were drafted following existing Dicio skill patterns and linguistic conventions, but native speaker review is recommended before merging to ensure natural phrasing in all languages.