-
-
Notifications
You must be signed in to change notification settings - Fork 255
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
Drop Ubuntu support from Ansible role #1416
Conversation
We added this very early in the project (tiny-pilot/ansible-role-tinypilot#77), and I don't think there are many users still on Ubuntu, if any. We dropped Ubuntu support for ansible-role-ustreamer six months ago (tiny-pilot/ansible-role-ustreamer#63), so I suspect that TinyPilot doesn't fully work on Ubuntu anyway. Removing Ubuntu support simplifies our Ansible logic and ultimately helps ease our transition away from Ansible.
Automated comment from CodeApprove ➜⏳ @jdeanwallace please review this Pull Request |
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.
Automated comment from CodeApprove ➜
⏳ Approval Pending (1 unresolved comments)
Approval will be granted automatically when all comments are resolved
Nice, LGTM!
In: Discussion
We dropped Ubuntu support for ansible-role-ustreamer six months ago (ansible-role-ustreamer#63), so I suspect that TinyPilot doesn't fully work on Ubuntu anyway.
Oh, we did? In that case, we still have conditional distribution logic lurking there too.
👀 @mtlynch it's your turn please take a look
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.
Automated comment from CodeApprove ➜
In: Discussion
Yeah, we can strip that logic out when we get to later stages of #1353.
I don't want to strip out useful logic too aggressively there until we fold it into the tinypilot because, in theory, someone else might want to take over that repo and build back cross-distro compatibility.
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.
Automated comment from CodeApprove ➜
Approved: I have approved this change on CodeApprove and all of my comments have been resolved.
We added this very early in the project (tiny-pilot/ansible-role-tinypilot#77), and I don't think there are many users still on Ubuntu, if any.
We dropped Ubuntu support for ansible-role-ustreamer six months ago (tiny-pilot/ansible-role-ustreamer#63), so I suspect that TinyPilot doesn't fully work on Ubuntu anyway.
This also simplifies the logic around writing to
/boot/config.txt
and/etc/modules
, which are Raspbian-specific paths. We partially added conditional logic for writing to these files for CI and partly to accommodate non-Raspbian distros. If we're not supporting other distros, I think a simpler approach is to more explicitly skip these steps in CI (under molecule).Removing Ubuntu support simplifies our Ansible logic and ultimately helps ease our transition away from Ansible.