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

Get projects after workspace has been already initialized #2867

Merged
merged 9 commits into from
Nov 3, 2016
Merged

Conversation

vzhukovs
Copy link
Contributor

This commit also includes API reorganization. Move client service implementations from the core-api to the core-app. Code cleanup. Removed dependency to the javax.inject from the core-api to avoid duplicate using @Inject annotation from the com.google.inject and javax.inject. (Client side uses only com.google.inject.Inject annotation)

Created two components:
WsAgentInitializer - which is responsible to initialize workspace component on the client side. Component consumes developer machine object at startup stage.
WsAgentMessageBusProvider - which is responsible to initialize message bus after workspace has been initialized.

API movements:
OAuthServiceClient was moved from the org.eclipse.che.ide.api.auth to the org.eclipse.ide.api.oauth

Related issue: #2629

@vparfonov @ashumilova @azatsarynnyy review it, please.

This commit also includes API reorganization. Move client service implementations from the core-api to the core-app. Code cleanup. Removed dependency to the javax.inject from the core-api to avoid duplicate using @Inject annotation from the com.google.inject and javax.inject. (Client side uses only com.google.inject.Inject annotation)

Created two components:
WsAgentInitializer - which is responsible to initialize workspace component on the client side. Component consumes developer machine object at startup stage.
WsAgentMessageBusProvider - which is responsible to initialize message bus after workspace has been initialized.

API movements:
OAuthServiceClient was moved from the org.eclipse.che.ide.api.auth to the org.eclipse.ide.api.oauth
@codenvy-ci
Copy link

Build # 768 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/768/ to view the results.

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

👍

* @see DevMachine
* @since 5.0.0
*/
public interface WsAgentInitializer {
Copy link
Member

Choose a reason for hiding this comment

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

As far as I understood this interface is used in IDE app only and not intended to be used by extensions.
So, if it isn't a part of IDE public API why do we need it in IDE API? May be it would be better to move it in IDE app module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right, I'll move it to the IDE app

@@ -139,7 +150,7 @@ public WsAgentState getState() {
return state;
}

/** Returns instance of {@link MachineMessageBus}. */
/** {@inheritDoc} */
public Promise<MessageBus> getMessageBus() {
Copy link
Member

Choose a reason for hiding this comment

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

@OverRide annotation is missed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

* @param devMachine
* development machine instance
*/
/** {@inheritDoc} */
public void initialize(DevMachine devMachine) {
Copy link
Member

Choose a reason for hiding this comment

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

@OverRide annotation is missed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -29,6 +31,7 @@
*
* @author Sergii Leschenko
*/
@Singleton
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add this annotation?
It's already bound as singleton in CoreGinModule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sure, fixed

* @param devMachine
* development machine instance
*/
/** {@inheritDoc} */
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of an empty javadoc which contains @inheritdoc tag only?
Javadoc tool and all popular IDEs override it automatically when generating documentation.
IMO we should add it only in case of 'real' overriding javadoc like here.
WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just personal habbit. I'll remove it.

/**
* @author Sergii Leschenko
*/
@Singleton
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add this annotation?
It's already bound as singleton in CoreGinModule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also fixed

@codenvy-ci
Copy link

Build # 770 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/770/ to view the results.

@codenvy-ci
Copy link

Build # 781 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/781/ to view the results.

Vladyslav Zhukovskii added 2 commits November 3, 2016 10:38
# Conflicts:
#	ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/project/ProjectServiceClientImpl.java
#	plugins/plugin-maven/che-plugin-maven-ide/src/main/java/org/eclipse/che/plugin/maven/client/comunnication/MavenMessagesHandler.java
@codenvy-ci
Copy link

@vzhukovs vzhukovs merged commit c7b4623 into master Nov 3, 2016
@vzhukovs vzhukovs deleted the che#2629 branch November 3, 2016 13:29
vzhukovs pushed a commit that referenced this pull request Nov 8, 2016
vzhukovs pushed a commit that referenced this pull request Nov 9, 2016
@bmicklea bmicklea mentioned this pull request Nov 16, 2016
66 tasks
@bmicklea bmicklea added this to the 5.0.0 milestone Jan 11, 2017
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
…e#2867)

* Get projects after workspace has been already initialized

This commit also includes API reorganization. Move client service implementations from the core-api to the core-app. Code cleanup. Removed dependency to the javax.inject from the core-api to avoid duplicate using @Inject annotation from the com.google.inject and javax.inject. (Client side uses only com.google.inject.Inject annotation)

Created two components:
WsAgentInitializer - which is responsible to initialize workspace component on the client side. Component consumes developer machine object at startup stage.
WsAgentMessageBusProvider - which is responsible to initialize message bus after workspace has been initialized.

API movements:
OAuthServiceClient was moved from the org.eclipse.che.ide.api.auth to the org.eclipse.ide.api.oauth

* Move WsAgentInitizalizer to the core-app

* Add @OverRide annotation

* Add @OverRide annotation

* Remove @singleton annotation

* Remove @singleton annotation

* Revert javax.inject.Inject annotation

* Update codebase due to changes in the master branch
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
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.

5 participants