-
Notifications
You must be signed in to change notification settings - Fork 18
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
refactor: using tasks directory instead of jobs #23
Conversation
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.
Thanks Emad! A couple comments, but I support the direction here.
# {{ cookiecutter.module_name }}/templates/{{ cookiecutter.plugin_name }}/tasks/ | ||
# and then add it to the MY_INIT_TASKS list. Each task is in the format: | ||
# ("<service>", ("<path>", "<to>", "<script>", "<template>")) | ||
MY_INIT_TASKS: list[tuple[str, tuple[str, ...]]] = [ | ||
# For example, to add LMS initialization steps, you could add the script template at: | ||
# {{ cookiecutter.module_name }}/templates/{{ cookiecutter.plugin_name }}/jobs/init/lms.sh | ||
# {{ cookiecutter.module_name }}/templates/{{ cookiecutter.plugin_name }}/tasks/{{ cookiecutter.plugin_name }}/init.sh | ||
# And then add the line: | ||
### ("lms", ("{{ cookiecutter.plugin_name }}", "jobs", "init", "lms.sh")), | ||
### ("lms", ("{{ cookiecutter.plugin_name }}", "tasks", "{{ cookiecutter.plugin_name }}", "init.sh")), |
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.
These changes aren't quite right. While, in general, the structure and names of these files are arbitrary (as long as the scripts are loaded into CLI_DO_COMMANDS
), the specific structure that the cookiecutter uses is this:
{{ cookiecutter.module_name }}/templates/{{ cookiecutter.plugin_name }}/tasks/SERVICE_NAME/JOB_NAME.sh
where:
- SERVICE_NAME is the service we're executing the task in. In the commented-out example, that's
lms
- JOB_NAME is the name of the job which this task is supporting. In this example, that's
init
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.
You're right. Do you have any particular service name in mind for this one?
also, should we drop the .sh
extension for the job? all of the official plugins use an init
file.
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.
@CodeWithEmad Let's use lms
, because that's the most common service in which plugins will need to execute tasks.
also, should we drop the .sh extension for the job? all of the official plugins use an init file.
Good question. I lean dropping the .sh
extension since that's what the official plugins do, but I'll leave it up to your discretion.
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 like the .sh
extension because it makes it easier to edit the files in the IDE.
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.
@CodeWithEmad , could you apply this change?
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.
Oh, I totally forgot about this. Absolutely.
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.
can you take a look now.
tasks directory in the cookie-cutter is called "jobs", but everywhere is "tasks". This will fix the inconsistency.
9ffb5e5
to
5ef6ec7
Compare
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.
Thanks @CodeWithEmad !
I've made sure that this will be in the soon-to-be-added Changelog, too.
Thanks Kyle. |
We are using a simple Changelog.md instead of using scriv because the cookiecutter doesn't have a versioned release process. Discussion at: #22 (comment) Includes a starter entry from: #23
tasks directory in the cookie-cutter is called "jobs", but everywhere is "tasks".
This will fix the inconsistency.