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

Removed deprecated activateTerminal method #10521 #10529

Conversation

abhisharsinha
Copy link
Contributor

@abhisharsinha abhisharsinha commented Dec 9, 2021

Signed-off-by: abhisharsinha [email protected]

What it does

Fixes: #10521

Removed the deprecated activateTerminal method. The deprecated method was called in two files which have been updated with the open method. The method definition was also removed.

How to test

No functionality was changed. The project builds without any error.

Review checklist

Reminder for reviewers

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution 👍

In order to accept your changes please be sure to sure to sign the eclipse contributor agreement (eca) with the same email as your authorship.

@abhisharsinha please be sure to also properly fill out the issue template which seems to have been discarded.

@abhisharsinha abhisharsinha marked this pull request as draft December 9, 2021 18:03
@abhisharsinha abhisharsinha marked this pull request as ready for review December 9, 2021 18:48
@abhisharsinha abhisharsinha reopened this Dec 9, 2021
@vince-fugnitto
Copy link
Member

@abhisharsinha it looks like an invalid email was used to author the pull-request which of course ultimately fails the CI check for the eca:

From c2a52e3369adce0637f4623140fa4303b89ea699 Mon Sep 17 00:00:00 2001
From: root <[email protected]>
Date: Thu, 9 Dec 2021 22:29:21 +0530
Subject: [PATCH] Closes #10521

While you're fixing the authorship, can you also please update the commit title and message itself to provide more details than "closes #" 👍

@vince-fugnitto vince-fugnitto added the quality issues related to code and application quality label Dec 10, 2021
@abhisharsinha abhisharsinha changed the title Closes #10521 Removed the deprecated activateTerminal method #10521 Dec 10, 2021
@abhisharsinha abhisharsinha changed the title Removed the deprecated activateTerminal method #10521 Removed deprecated activateTerminal method #10521 Dec 10, 2021
@abhisharsinha abhisharsinha marked this pull request as draft December 10, 2021 15:22
@abhisharsinha abhisharsinha force-pushed the removed_activateTerminal_method branch from 9ef28e6 to 7c3a82a Compare December 10, 2021 15:41
@abhisharsinha abhisharsinha marked this pull request as ready for review December 10, 2021 15:41
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@abhisharsinha the changes look good to me, do you mind adding a breaking change note in the
changelog to mentioned that we now removed the deprecated method?

abhisharsinha added a commit to abhisharsinha/theia that referenced this pull request Dec 10, 2021
Deprecated method `activateTerminal` was removed.
@abhisharsinha
Copy link
Contributor Author

@vince-fugnitto This was my first open-source contribution. Thank you for guiding me through it.

abhisharsinha added a commit to abhisharsinha/theia that referenced this pull request Dec 10, 2021
@vince-fugnitto
Copy link
Member

@abhisharsinha no problem! 👍 everything looks good, do you mind as one final step to squash your commits so only one is present?

@abhisharsinha abhisharsinha force-pushed the removed_activateTerminal_method branch from 0e59bc9 to 963b2a4 Compare December 10, 2021 21:11
@abhisharsinha abhisharsinha marked this pull request as draft December 10, 2021 21:54
@abhisharsinha abhisharsinha force-pushed the removed_activateTerminal_method branch from 164d672 to 963b2a4 Compare December 10, 2021 22:25
@abhisharsinha abhisharsinha marked this pull request as ready for review December 11, 2021 11:37
@vince-fugnitto
Copy link
Member

@abhisharsinha do you need any help to do the rebase?

@abhisharsinha
Copy link
Contributor Author

@abhisharsinha do you need any help to do the rebase?

Yes. I had squased the commits but there was a merge conflict. I am unable to squash merge commit.

@vince-fugnitto vince-fugnitto force-pushed the removed_activateTerminal_method branch from cfdfc32 to 82672cf Compare December 13, 2021 16:05
@vince-fugnitto vince-fugnitto force-pushed the removed_activateTerminal_method branch from 82672cf to 3e0b867 Compare December 13, 2021 16:05
@vince-fugnitto
Copy link
Member

@abhisharsinha I cleaned it up for you 👍 I'll wait for CI to pass.

@vince-fugnitto vince-fugnitto merged commit 12a742f into eclipse-theia:master Dec 13, 2021
@vince-fugnitto vince-fugnitto added this to the 1.21.0 milestone Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality issues related to code and application quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terminal: remove deprecated activateTerminal method
2 participants