-
Notifications
You must be signed in to change notification settings - Fork 470
feat: 4107 onboarding use case selection #4539
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c341e60
02d79db
fc3768b
998ef13
eb803d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same question on this test file as well
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mnafees Mostly the same answer as above. However, this file belongs in the CLI because it tests that the released CLI generates the project, starts the worker, and completes the trigger. While content edits do not touch this file either, adding or removing a combination currently does require a one-line list update. With my proposed manifest follow-up PR its lists can be derived from the manifest, i.e. that maintenance is removed. |
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.
What's the use case of having this test? To me it seems like every time we make changes to the quickstart repo / examples we will need to keep updating this which seems like some wasted work?
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.
@mnafees thanks for the feedback. I wanted to clarify a few things and suggest a design improvement for a follow-up PR.
If the quickstart repo's implementation changes but the contract does not, the only hatchet repo update is bumping
go.modandgo.sumto the new release. Have a look at the recentv0.3.1bump in this PR (fc3768b) as an example. It changed the scheduled TypeScript template's input handling, and the only edits here in that commit are a test rename and comment cleanup.The intent of
selection_testis to validate what the CLI depends on, which use cases and languages exist and which selections validate. The CLI automatically discovers which use cases the embedded quickstarts includes. In contrast, new languages are not automatically discovered, because the CLI hardcodes the language list and each language's package managers. The assertions only catch removals. Without them if a release dropped a language from a use case, the CLI would silently stop offering it while the dashboard onboarding would continue to print commands for it.The design should be improved. If hatchet-quickstarts published a manifest of use cases, languages, and package managers alongside the templates, the CLI could derive everything from it and any tests would move to the quickstarts repo where the changes actually happen. Neither adding nor removing a language would require CLI code change. That is a design change though, so I would like to implement it in a follow-up PR.