Skip to content
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

Clean-up cell output debt #105846

Closed
3 tasks done
jrieken opened this issue Sep 1, 2020 · 8 comments
Closed
3 tasks done

Clean-up cell output debt #105846

jrieken opened this issue Sep 1, 2020 · 8 comments
Assignees
Labels
api debt Code quality issues notebook under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Sep 1, 2020

Some debt has accumulated around cell output and we should start to clean things up

  • there is transformEditsOutputs and transformSpliceOutputs which feel like they are on the wrong layer, e.g should this be part of the serialisation of output
  • similar to transfromOutputs is IProcessedOutput which also seems wrongly placed. Maybe the internal model object specs all the data of those two and the API types are directly converted into those types
  • the proposed API is very rough, esp the outputKind-marker is an api-alien concept
@jrieken jrieken added debt Code quality issues notebook under-discussion Issue is under discussion for relevance, priority, approach labels Sep 1, 2020
@jrieken jrieken added this to the September 2020 milestone Sep 1, 2020
@jrieken jrieken added the api label Sep 15, 2020
@jrieken jrieken modified the milestones: September 2020, October 2020 Sep 28, 2020
@rebornix
Copy link
Member

rebornix commented Oct 5, 2020

This one relates the renderer work @connor4312 is doing now. If the output renderers are all pure and lives in the webview, then we don't need IProcessedOutput or transform*Outputs at all as there is no back and forth between the UI and ExtHost, raw outputs will be the source of truth.

@connor4312
Copy link
Member

Yea, I can clean these up this month.

@jrieken jrieken modified the milestones: October 2020, November 2020 Oct 23, 2020
@jrieken
Copy link
Member Author

jrieken commented Nov 17, 2020

@connor4312 Can you/we tackle those debt items before going all-in on the new API and such?

@jrieken
Copy link
Member Author

jrieken commented Nov 18, 2020

@rebornix @roblourens I have created #110855 which removes the transform logic and moves that into a single place

@connor4312
Copy link
Member

Sure, I should have time this month to tack some of that

@connor4312
Copy link
Member

Hi, finally have some unallocated time to do this 🙂

Before I just in, I know you have been working on hammering out the final versions of these APIs. Does that work affect this cleanup, or can I go ahead and take care of this tomorrow?

@jrieken
Copy link
Member Author

jrieken commented Feb 5, 2021

Please hold back or join us in #115817

@jrieken
Copy link
Member Author

jrieken commented Feb 17, 2021

We are done with this

@jrieken jrieken closed this as completed Feb 17, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api debt Code quality issues notebook under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

3 participants