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

Replaced all unnecessary 'private' modifiers with 'protected' in Work… #9597

Merged
merged 1 commit into from
Jun 21, 2021
Merged

Replaced all unnecessary 'private' modifiers with 'protected' in Work… #9597

merged 1 commit into from
Jun 21, 2021

Conversation

jyggiz
Copy link

@jyggiz jyggiz commented Jun 16, 2021

What it does

Fixes: #9591

Removes the optional use of 'private' modifiers in Workspace Service to make it easier to write code for child classes in the future.

How to test

  1. This is more of a code refactor, so you can just run the application and try to use the features associated with the Workspace Service (For example, open a new workspace and check if there are any errors in the logs related to new changes)

Review checklist

Reminder for reviewers

Signed-off-by: Timur Zhigmytov [email protected]

@vince-fugnitto vince-fugnitto added quality issues related to code and application quality workspace issues related to the workspace labels Jun 16, 2021
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.

@jyggiz thank you for your first contribution to the project 👍

Do you mind squashing your commits, removing the merge commit as well so it can be accepted? In addition, can you update the commit title (currently too long) and message so it includes information as to why the change is necessary (private to protected in order to allow easier extensibility).

@jyggiz
Copy link
Author

jyggiz commented Jun 17, 2021

@vince-fugnitto I removed unnecessary commits and changed the description!

@jyggiz jyggiz requested a review from vince-fugnitto June 18, 2021 12:07
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.

The changes look good to me 👍

Copy link
Contributor

@kenneth-marut-work kenneth-marut-work left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for taking care of this 🙂

@kenneth-marut-work kenneth-marut-work merged commit 6afae9e into eclipse-theia:master Jun 21, 2021
@vince-fugnitto vince-fugnitto added this to the 1.15.0 milestone Jun 30, 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 workspace issues related to the workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workspace service contains private modifiers
3 participants