Skip to content

Get rid of the configurator workflow design pattern#47

Merged
seratch merged 2 commits intomainfrom
all-resources
Feb 20, 2025
Merged

Get rid of the configurator workflow design pattern#47
seratch merged 2 commits intomainfrom
all-resources

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Feb 19, 2025

Type of change (place an x in the [ ] that applies)

  • New sample
  • New feature
  • Bug fix
  • Documentation

Summary

This pull request significantly simplifies the way to run this app. Users no longer need to manage the list of channel IDs within a workflow trigger.

Requirements (place an x in each [ ] that applies)

  • I’ve checked my submission against the Samples Checklist to ensure it complies with all standards
  • I have ensured the changes I am contributing align with existing patterns and have tested and linted my code
  • I've read and agree to the Code of Conduct

@seratch seratch added the enhancement New feature or request label Feb 19, 2025
@seratch seratch self-assigned this Feb 19, 2025
Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I found the app setup convenient with these steps handled by the event trigger - this is dang neat 👾

One comment related to all_resources in the reaction_added_trigger.ts might be alright to remove too, but this isn't a blocker at all. These translating threads interesting as is! 👏 ✨

Comment on lines +150 to +152
Once this app's bot user is added to a channel, adding reactions such as `:jp:`
and `:fr:` results in posting translation results of the target message as
replies in its thread.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This is a really slick setup 🚢 💨 ✨

Comment on lines -45 to -49
// configurator
"triggers:read",
"triggers:write",
"channels:join",
"groups:read",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🫡

*/
export default Manifest({
name: "DeepL Translator",
name: "DeepL Translator 2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of great improvements all throughout! 🚀

@seratch
Copy link
Contributor Author

seratch commented Feb 20, 2025

@zimeg Thanks for the swift review as always and great catch on the comments! I've removed them, so let me merge this now.

@seratch seratch merged commit 4ba9fb2 into main Feb 20, 2025
2 checks passed
@seratch seratch deleted the all-resources branch February 20, 2025 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants