-
Notifications
You must be signed in to change notification settings - Fork 17
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
Allow register different viewers for different WorkChain. #541
Conversation
I think this will need some discussion, since it would be a breaking change? |
3801b3d
to
74ef2a1
Compare
Now it has backward capacity. I agree we need more discussion on it. |
Thanks @superstar54. Can you take a look at the failing tests? Looks like the failures might not be related to this PR, but should be fixed regardless. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #541 +/- ##
==========================================
+ Coverage 96.17% 96.21% +0.03%
==========================================
Files 11 11
Lines 1177 1188 +11
==========================================
+ Hits 1132 1143 +11
Misses 45 45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The failed test is caused by a update in the template in |
@superstar54 thank you for fixing the test. Could you please make a separate PR with that fix? I assume the test failures would pop up on other PRs as well? Does this PR need to be merged for 2.1.0 version? |
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've thought about this a bit more and it seems like a good feature to have.
The only thing missing here is a unit test for the new behaviour.
Hi @danielhollas , I added a unit test for this behavior. Could you please have 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.
@superstar54 perfect, thanks!
Fix #540 , use
process_type
as the node viewer's key.Here is an example, aiidalab/aiidalab-qe#559