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

VS Code Extension: Architecture improvements - Milestone 1 #39

Merged

Conversation

eburnette
Copy link
Contributor

VS Code Extension: Architecture improvements - Milestone 1

Description

This PR is for issue #3

Milestone 1 describes the planned architecture changes. It contains no code.

Submission Links & Documents

N/A

Requirements Check

  • Have have you met the milestone requirements? YES
  • Have you included tests (if applicable)? N/A
  • Have you met the contribution guidelines of the repos you have submitted code to (if applicable)? N/A

Other Details

  • Is there anything specific you'd like the PoC to know or review for? NO
  • Are there other references, documentation, or relevant artificats to mention for this PR (ie. external links to justify design decisions, etc.)? NO

@sideninja
Copy link
Member

Should this be converted to draft if it's still WIP?

@eburnette
Copy link
Contributor Author

It's complete unless there are comments that require changes.

@srinjoyc srinjoyc added this to Milestone 1 - Complete in FLIP Fest Team Progress Sep 23, 2021
Copy link
Contributor

@psiemens psiemens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks @eburnette!

I'll let @sideninja weigh in too, but you're good to proceed to milestone 2.

@srinjoyc srinjoyc changed the title Milestone 1 initial submission VS Code Extension: Architecture improvements - Milestone 1 Sep 27, 2021
@psiemens
Copy link
Contributor

Merged @eburnette!

@psiemens psiemens merged commit 7217fc9 into onflow:main Sep 28, 2021
@sideninja
Copy link
Member

sideninja commented Sep 28, 2021

Hi, @eburnette I overlooked this document as I thought it is linking the PR on the vscode.
I think your plan looks solid, the only concern I have is about extracting the flowkit to a separate repository, although I agree this is a good decision down the line I don't think it is mature enough to be treated as a separate component, reasons being as soon as we extract it and advertise it as a package to be used by others we need to take extra care with changes in the API as for now it is more of a draft version and after we learn what is the best API from the LS and other implementations we can then think about extracting it. This extraction should it happen as part of this task it should be part of extra features in the end and we need to discuss documentation, API design, etc. But I agree 100% and my plan was too to extract it at some point, I just don't feel the API is mature enough. cc @psiemens

@eburnette
Copy link
Contributor Author

@sideninja will there be any problem with the extension reaching into the cli project to get flowkit? If not, then I agree the repo change can wait.

@sideninja
Copy link
Member

@eburnette no problem at all, in fact there is already flowkit used in the LS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
FLIP Fest Team Progress
Milestone 1 - Complete
Development

Successfully merging this pull request may close these issues.

None yet

3 participants