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

CHE-998. Add 'Reimport' action for maven project #1277

Merged
merged 1 commit into from
May 24, 2016
Merged

Conversation

RomanNikitenko
Copy link
Member

@RomanNikitenko RomanNikitenko commented May 17, 2016

@mmorhun
Copy link
Contributor

mmorhun commented May 17, 2016

Great! But imo, ''New" menu item should be the first. Reimport is more rare case then add file or package.

@RomanNikitenko
Copy link
Member Author

@mmorhun Done!

@codenvy-ci
Copy link

Build # 603 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/603/ to view the results.

@@ -105,8 +106,10 @@ private void registerDefaultActionGroups() {
DefaultActionGroup mainContextMenuGroup = new DefaultActionGroup(IdeActions.GROUP_MAIN_CONTEXT_MENU, false, this);
registerAction(IdeActions.GROUP_MAIN_CONTEXT_MENU, mainContextMenuGroup);

DefaultActionGroup buildContextMenuGroup = new DefaultActionGroup(IdeActions.GROUP_BUILD_CONTEXT_MENU, false, this);
DefaultActionGroup buildContextMenuGroup = new DefaultActionGroup("Maven", true, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maven shouldn't be a constant ?

Copy link
Member Author

@RomanNikitenko RomanNikitenko May 18, 2016

Choose a reason for hiding this comment

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

@benoitf We have the same behavior here: https://github.com/eclipse/che/blob/19162869b908fb61794518a3765a47a5380d7158/core/ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/actions/ActionManagerImpl.java#L77-L101
I think we should have the consistent behavior.
So should we use constants in all places in this class?
@vparfonov WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I think that constant for group's name isn't necessary. Because it used in one place only.

@slemeur
Copy link
Contributor

slemeur commented May 23, 2016

OK, alignements to be fixed in separate issue

@RomanNikitenko
Copy link
Member Author

@vparfonov I've updated pull request, please review.

assistantGroup.add(getEffectivePomAction, Constraints.LAST);

// create maven context menu
DefaultActionGroup mavenContextMenuGroup = new DefaultActionGroup("Maven", true, actionManager);
actionManager.registerAction("mavenContextMenu", mavenContextMenuGroup);
Copy link
Contributor

Choose a reason for hiding this comment

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

action should be a constant

@vparfonov
Copy link
Contributor

ok

@codenvy-ci
Copy link

Signed-off-by: Roman Nikitenko <rnikitenko@codenvy.com>
This pull request was closed.
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.

7 participants