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

[INTERNAL] Remove IWorkspaceRoot #950

Merged
merged 2 commits into from
May 6, 2020
Merged

Conversation

tobous
Copy link
Member

@tobous tobous commented Apr 29, 2020

Removes the interface IWorkspaceRoot as it was no longer being used.

Removes the IntelliJ implementation of IWorkspaceRoot.

Removes the interface declaration from EclipseWorkspaceRoot. This
implementation remains as it is still used when requesting the parent
container of a resource located in the workspace root.

@tobous tobous self-assigned this Apr 29, 2020
@tobous
Copy link
Member Author

tobous commented Apr 29, 2020

Instead of keeping EclipseWorkspaceRootImpl around, I could remove the abstract modifier from EclipseContainerImpl. Any opinions? (Otherwise, I am just going to leave it the way it is. 😉)

srossbach
srossbach previously approved these changes May 3, 2020

/** Eclipse implementation of {@link IWorkspaceRoot}. */
public class EclipseWorkspaceRootImpl extends EclipseContainerImpl implements IWorkspaceRoot {
/** Eclipse {@link IContainer} implementation representing an {@link IWorkspaceRoot}. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this class deleted in another patch ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I didn't remove it because it is still used in the eclipse implementation. If a file in the workspace root asks for its parent resource, a workspace root object is returned instead of a folder. This class is now used as a simple implementation of the abstract EclipseContainerImpl. But I could also remove the abstract modifier from EclipseContainerImpl and change the delegate type to IContainer instead of IReosurce if you would prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I could also remove the abstract modifier from EclipseContainerImpl and change the delegate type to IContainer instead of IReosurce if you would prefer

Do you think the additional abstraction layer EclipseWorkspaceRootImpl provides valuable information? Otherwise, I would prefer your idea of removing the class and using the EclipseContainerImpl.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think the additional abstraction layer EclipseWorkspaceRootImpl provides valuable information?

No, not really as I couldn't be bothered to added any new javadoc to it. Will adjust the container implementation and remove the class.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@tobous
Copy link
Member Author

tobous commented May 4, 2020

Rebased onto current master fixing minor merge conflicts.

@tobous
Copy link
Member Author

tobous commented May 5, 2020

Removed 'abstract' modifier from EclipseContainerImpl. Removed EclipseWorkspaceRootImpl. Directly used EclipseContainerImpl instead.

m273d15
m273d15 previously approved these changes May 5, 2020
tobous added 2 commits May 7, 2020 00:35
Removes the interface IWorkspaceRoot as it was no longer being used.

Removes the IntelliJ implementation of IWorkspaceRoot.

Removes the interface declaration from EclipseWorkspaceRoot. This
implementation remains as it is still used when requesting the parent
container of a resource located in the workspace root.
Removes the abstract modifier from the class EclipseContainerImpl.

Uses EclipseContainerImpl as a replacement for EclipseWorkspaceRootImpl
as the class no longer contained any methods.

Removes EclipseWorkspaceRootImpl.
@tobous
Copy link
Member Author

tobous commented May 6, 2020

Rebased onto current master without any interaction.

@tobous tobous merged commit a456ed8 into master May 6, 2020
@tobous tobous deleted the pr/core/remove-iworkspaceroot branch May 6, 2020 22:41
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.

None yet

3 participants