Skip to content

Conversation

jeffmaury
Copy link
Member

Signed-off-by: Jeff MAURY jmaury@redhat.com

…le for main project fails

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
@jeffmaury jeffmaury requested a review from adietish June 25, 2018 08:34
MavenProject mavenProject = facade.getMavenProject(monitor);
ModuleCoreNature.addModuleCoreNatureIfNecessary(facade.getProject(), monitor);
addUtilityFacet(mavenProject, facade.getProject(), monitor);
if (ModuleUtils.hasModuleCoreNature(project)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd think that this check isn't required any longer. Or if it is we should most likely error (what else than an error is it if we set the core nature and then the check for it 2 lines further down fails?).

Copy link
Member Author

Choose a reason for hiding this comment

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

I was already there and as it was about the root project, I did not want to modify but can remove it if you say so

public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException {
try {
MavenProject mavenProject = facade.getMavenProject(monitor);
ModuleCoreNature.addModuleCoreNatureIfNecessary(facade.getProject(), monitor);
Copy link
Member

Choose a reason for hiding this comment

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

if we put ModuleCoreNature.addModuleCoreNature into #addUtilityFacet we'd have a single occurrence where module core is added (versus 2 right now). WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
@adietish
Copy link
Member

@jeffmaury thx!

@adietish adietish merged commit fa96b53 into jbosstools:master Jun 25, 2018
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.

2 participants