-
Notifications
You must be signed in to change notification settings - Fork 34
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
🔖 Release 3.5.2 #353
🔖 Release 3.5.2 #353
Conversation
ayushuk
commented
May 18, 2024
- 🧹Remove Okapilib as a default template for PROS 4 #351
- 🧹 Remove PROS 4 Prompts #346
- 🚸 Apply liblvgl when upgrading to PROS 4 #345
* Apply liblvgl when upgrading from PROS 3 to PROS 4 * Fetch template if necessary
* Update version numbers * 🐛 Fix default template selection #318 * Update version numbers * Update version numbers --------- Co-authored-by: BennyBot <[email protected]>
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.
One comment. if it is tested and won't be an issue, LGTM and I will apprrove
@@ -159,7 +159,7 @@ def upgrade(ctx: click.Context, project: c.Project, query: c.BaseTemplate, **kwa | |||
""" | |||
analytics.send("upgrade-project") | |||
if not query.name: | |||
for template in project.templates.keys(): | |||
for template in tuple(project.templates.keys()): |
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.
Just to be sure, this won't break if they have old conductor files?
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.
From my understanding, this function doesn't touch the conductor file (there are no .save()
calls).
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.
LGTM