Open seenanair opened 2 weeks ago
Attention: Patch coverage is 94.05099%
with 21 lines
in your changes missing coverage. Please review.
Project coverage is 80.43%. Comparing base (
abd7f03
) to head (aaf4457
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚨 Try these New Features:
Looks good, nice work!
I don't know how to test this manually but I am assuming you have and I reckon the int suite will cover most of the use cases.
That’s something I’m also a bit concerned about. I hope the integration suite covers most of it, and I’ll also ask the team if there are any better ways to test it.
Nice work, I like the direction this is taking @seenanair! I see you've found the eventBus too, hopefully we can eventually get rid of all the script tags 🤞
Nice work, I like the direction this is taking @seenanair! I see you've found the eventBus too, hopefully we can eventually get rid of all the script tags 🤞
Thanks @StephenHulme for the review. 🙏 The asset comment components were pretty tightly linked before, assuming all of them would always be on the same page in Limber. That worked fine for how things were currently set up in Limber, but I hope iit made sense to decouple them so they can be used independently if needed down the line. Plus, this way, we can avoid relying on data altogether.
This would be a great case for proper store management, but that felt like a big step right now. So, I just used the event bus since it was already there and working fine.
Closes https://github.com/sanger/limber/issues/2033
Changes proposed in this pull request
Additional Notes There is potential to globally register other shared components. However, some of these components are currently registered with different names across the application. A major refactor would be needed to unify these, which we recommend handling in a separate story to avoid an overly complex PR.
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main
]- Check story numbers included
- Check for debug code
- Check version