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

Revert "[VSC] Initialize vscode plugin" #1112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

srossbach
Copy link
Contributor

Reverts #1111

@Drakulix
Copy link
Contributor

Sorry for the premature merge of #1111.
I guess we can skip the review process for this, as it is just a plain revert. Any objections?

@srossbach
Copy link
Contributor Author

Seems reopening the original pull request is not that trivial. I do not want to add a revert of the revert. Please stop merging stuff next time while there is a still a discussion pending. Of course we could request changes but the discussion was more about a "Machbarkeitsstudie" rather than regarding the changes made in the patch.

@Drakulix
Copy link
Contributor

Seems reopening the original pull request is not that trivial. I do not want to add a revert of the revert. Please stop merging stuff next time while there is a still a discussion pending. Of course we could request changes but the discussion was more about a "Machbarkeitsstudie" rather than regarding the changes made in the patch.

I genuinely overlooked the discussion as I meant to merge this days ago (before the discussion), but just forgot. Today I remembered, everything was still green and I simply did not notice the new comments. I would suggest next time, if there are such big issues with the general concept, a negative review would still be helpful. Still I should have catch'ed up and definitely not merged it. Sorry about that.

@tobous
Copy link
Member

tobous commented Jan 6, 2021

While the PR was merged prematurely, I don't think it is necessary to revert it. It only adds basic build scripting and documentation. None of these are affected by the discussed issues. And in case the discussed issues prove to be insurmountable, everything relating to the VSCode implementation (including the LSP stuff) will probably be deleted anyways, meaning just reverting this part would have been unnecessary. So this PR can be closed in my opinion.

But the discussion started in the merged PR definitely should be continued to better assess the viability of a VSCode/LSP-based implementation of Saros.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants