-
Notifications
You must be signed in to change notification settings - Fork 122
[v0/v1 migration] Move facet metadata enrichment to V2 #6104
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
Changes from 7 commits
43629f0
16a7952
6839c01
1fa4087
623bb59
cc014e1
9adeab4
2eed638
5e2778e
d4ce45f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overall, the implementation looks good to me! This file is getting quite long though. What do you think of pulling out the helper functions into their own file, and leaving just the bp.route() functions here? Totally fine to leave this for a followup PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a good call (and something I was pondering as well). I've put it in as a TODO (along with the other item I flagged) for a separate PR. |
Large diffs are not rendered by default.
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.
Overall, the implementation looks good to me! This file is getting quite long though. What do you think of pulling out the helper functions into their own file, and leaving just the bp.route() functions here?