Skip to content
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

feat: expose integer Tutor version parts to templates #1060

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

kdmccormick
Copy link
Collaborator

@kdmccormick kdmccormick commented May 6, 2024

@michaelwheeler
Copy link
Contributor

For some context, this is related to a question that I brought up during the Tutor Users' Group about writing plugins that support multiple Tutor versions. We're currently doing something along the lines of...

{% if TUTOR_VERSION.startswith("14.") %}
# Do something for Tutor v14 only...
{% else %}
# Do something for Tutor versions after 14...
{% endif %}

But having major/minor/patch versions available to the context as integers would allow more precision and flexibility.

@kdmccormick kdmccormick self-assigned this May 15, 2024
@regisb
Copy link
Contributor

regisb commented Jun 10, 2024

@michaelwheeler I'd like to understand better your use case. Have you considered having multiple branches for your plugin, one for every major release?

@michaelwheeler
Copy link
Contributor

Have you considered having multiple branches for your plugin, one for every major release?

In our team, managing multiple branches for our plugins felt like more overhead than adding an occasional conditional. We also think it will slightly simplify the process of upgrading to a newer named release.

@arbrandes
Copy link

I think this would useful to have as an option for plugin authors, particularly for cases where the plugins don't change that much between releases.

@regisb
Copy link
Contributor

regisb commented Jun 14, 2024

Right. I'm still not sure if it makes sense to add these settings to Tutor core. You do know that you could define these variables in your own plugin by adding a callback to the ENV_TEMPLATE_VARIABLES filter, right?

@michaelwheeler
Copy link
Contributor

You do know that you could define these variables in your own plugin by adding a callback to the ENV_TEMPLATE_VARIABLES filter, right?

I didn't know that!

It still feels to me like Tutor should provide the variable, since the version is a property of Tutor itself, but if that's undesirable for other reasons then maybe ENV_TEMPLATE_VARIABLES can get me where I need to go.

tutor/env.py Outdated Show resolved Hide resolved
@kdmccormick kdmccormick force-pushed the kdmccormick/tutor-version-parts branch 4 times, most recently from ee9b065 to 6c2faae Compare November 7, 2024 21:57
@kdmccormick kdmccormick force-pushed the kdmccormick/tutor-version-parts branch from 6c2faae to 0758f55 Compare November 7, 2024 21:59
@kdmccormick
Copy link
Collaborator Author

kdmccormick commented Nov 7, 2024

Sorry for the delay @michaelwheeler. I think this is ready to go. Can you review it and try it out with one of your plugins?

@michaelwheeler
Copy link
Contributor

I think this is ready to go. Can you review it and try it out with one of your plugins?

Yep, thanks! I'll try to take a look before the end of next week.

@michaelwheeler
Copy link
Contributor

Hey @kdmccormick, I tested this out and things work as expected from my perspective. Thanks!

@kdmccormick kdmccormick marked this pull request as ready for review November 16, 2024 00:05
@kdmccormick
Copy link
Collaborator Author

@michaelwheeler Would you have any objection to me removing TUTOR_VERSION_PATCH from this PR? You'd still have TUTOR_VERSION_MAJOR and TUTOR_VERSION_MINOR, but the patch version would remain unavailable to plugin authors without manually parsing TUTOR_VERSION.

My thinking is that it would be nice, as maintainers, to have some flexibility with the patch version. We don't currently use alpha/beta suffixes or release candidates (19.2.3a, 19.2.rc1, etc.) but I can't say for sure that we never will, so I'd rather not encourage plugin authors to rely on it always being an integer.

@michaelwheeler
Copy link
Contributor

Would you have any objection to me removing TUTOR_VERSION_PATCH from this PR?

No objection @kdmccormick. In practice I suspect I would only ever be using TUTOR_VERSION_MAJOR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants