Closed klauer closed 2 years ago
For this PR to be ready-for-review:
_pyqt_designer_
settings such that they go to the appropriate "Typhos Widgets" groupThis is technically ready, but it doesn't make much sense to start the review process when we want to wait on an upstream PyDM tag
PyDM v1.16.1 is out: https://github.com/slaclab/pydm/releases/tag/v1.16.1 Marking this ready for review - will bump the required pydm version in an upcoming commit.
As a note for testing: isolated testing here would be advisable, but what matters most (of course!) is our final inclusion into pcds-envs.
This looks good to me, and I like the idea of it a lot. I don't have designer set up yet, so that part of my review will be delayed. (hopefully not too long)
CI failed due to a timeout on py3.8 specifically. I restarted the job but it looks to be a persistent problem.
I'm (fortunately) seeing the hang here locally:
typhos/tests/test_suite.py::test_suite_save_util PASSED [ 85%]:
All of the updates here are good and well-motivated The test suite hanging is perplexing and doesn't seem to be in the same place every time
Force-pushed to remove deprecation fixes and isolate this enhancement proposal
It hasn't significantly changed, @ZLLentz, but it's still segfaulting randomly
@klauer do we need to run this a few times until we catch the segfault? All the other typhos builds are passing consistently now (and this one even passed this time)
@ZLLentz yes, that would be advisable
Should we merge this, then? You've eliminated all the CI issues
I think if all looks good here, we should probably merge. I admittedly didn't dig back into the code to re-self-review, I was just hoping that the CI would be fixed...
Edit: worth reiterating for the next tag:
conda-forge recipe will need updating
Description
Upstream PR: https://github.com/slaclab/pydm/pull/861
Motivation and Context
Activation scripts are painful to write, maintain, and even use sometimes. Let's remove an additional source of failure and rely on PyDM where it's sensible to.
Closes #504
How Has This Been Tested?
Interactively
Where Has This Been Documented?
This PR text
Screenshots (if appropriate):
With the groups set as of 4cf5612: