-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
output/{json,csv}: Metadata migration #2727
Conversation
195c1d3
to
c8fa900
Compare
I just added in the fixups the simplest solution for having this, it requires more refactoring and polishing, I will do them if we will have a consensus. I'm not sure I like this proposal based on adding a fixed naming when we still haven't a consensus on the JS API. |
1ce73e9
to
3c97ec7
Compare
c8fa900
to
42db878
Compare
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.
LGTM, thank you! 🙇
42db878
to
2ba0e46
Compare
Rebased, I tried to refactor a bit but it touches too many code and it doesn't sound a good idea for v0.41.0. I will keep them for a dedicated PR in v0.42.0 |
It continues the work started in #2654, we decided to split for having a scoped PR and discuss better the implementation details.