[MNG-6203] Minor cleanup in MavenCli.java

[jira-importer]

Stefan Eicher opened MNG-6203 and commented

There seems to be some old unnecessary code in the MavenCli.java
that could be cleaned up.

---

Affects: 3.5.0-beta-1

Attachments:

• Bildschirmfoto 2017-04-02 um 11.26.40.png (57.02 kB)
• Bildschirmfoto 2017-04-02 um 11.37.41.png (84.82 kB)

Remote Links:

• GitHub Pull Request #111
  

[jira-importer]

Karl Heinz Marbaise commented

Why not posting this code snippets into the description instead of attaching screen shots? Apart from that maybe I misunderstand a thing here? Which code is unnecessary?

[jira-importer]

Stefan Eicher commented

Incredible how fast you were! I was just interrupted by my wife and could not finish the issue. I posted a printscreen that you can see the code line...

I would propose to remove this part since userToolchainsFile is never used:

File userToolchainsFile;
        if ( commandLine.hasOption( CLIManager.ALTERNATE_USER_TOOLCHAINS ) )
        {
            userToolchainsFile = new File( commandLine.getOptionValue( CLIManager.ALTERNATE_USER_TOOLCHAINS ) );
            userToolchainsFile = resolveFile( userToolchainsFile, workingDirectory );
        }
        else
        {
            userToolchainsFile = MavenCli.DEFAULT_USER_TOOLCHAINS_FILE;
        }

It seems as this code once has been moved to here:

 private void toolchains( CliRequest cliRequest )
        throws Exception
    {
        File userToolchainsFile;

        if ( cliRequest.commandLine.hasOption( CLIManager.ALTERNATE_USER_TOOLCHAINS ) )
        {
            userToolchainsFile =
                new File( cliRequest.commandLine.getOptionValue( CLIManager.ALTERNATE_USER_TOOLCHAINS ) );
            userToolchainsFile = resolveFile( userToolchainsFile, cliRequest.workingDirectory );

            if ( !userToolchainsFile.isFile() )
            {
                throw new FileNotFoundException(
                    "The specified user toolchains file does not exist: " + userToolchainsFile );
            }
        }

Gruss Stefan

[jira-importer]

Karl Heinz Marbaise commented

I was just that fast cause I was at my computer at this moment.....furthermore if you take a look some lines down you will see that userToolchainsFile is used:

        File userToolchainsFile;

        if ( cliRequest.commandLine.hasOption( CLIManager.ALTERNATE_USER_TOOLCHAINS ) )
        {
            userToolchainsFile =
                new File( cliRequest.commandLine.getOptionValue( CLIManager.ALTERNATE_USER_TOOLCHAINS ) );
            userToolchainsFile = resolveFile( userToolchainsFile, cliRequest.workingDirectory );

            if ( !userToolchainsFile.isFile() )
            {
                throw new FileNotFoundException(
                    "The specified user toolchains file does not exist: " + userToolchainsFile );
            }
        }
        else
        {
            userToolchainsFile = DEFAULT_USER_TOOLCHAINS_FILE;
        }

        File globalToolchainsFile;

        if ( cliRequest.commandLine.hasOption( CLIManager.ALTERNATE_GLOBAL_TOOLCHAINS ) )
        {
            globalToolchainsFile =
                new File( cliRequest.commandLine.getOptionValue( CLIManager.ALTERNATE_GLOBAL_TOOLCHAINS ) );
            globalToolchainsFile = resolveFile( globalToolchainsFile, cliRequest.workingDirectory );

            if ( !globalToolchainsFile.isFile() )
            {
                throw new FileNotFoundException(
                    "The specified global toolchains file does not exist: " + globalToolchainsFile );
            }
        }
        else
        {
            globalToolchainsFile = DEFAULT_GLOBAL_TOOLCHAINS_FILE;
        }

        cliRequest.request.setGlobalToolchainsFile( globalToolchainsFile );
        cliRequest.request.setUserToolchainsFile( userToolchainsFile );

Kind regards
Karl Heinz

[jira-importer]

Stefan Eicher commented

I was talking of the File userToolchainsFile; in line 1465 not the one in 1215

[jira-importer]

Karl Heinz Marbaise commented

Ah Ok...wrong location....Now I understand your comments...oh my god...I have mistaken this sorry...

[jira-importer]

Karl Heinz Marbaise commented

Hm..

    private MavenExecutionRequest populateRequest( CliRequest cliRequest, MavenExecutionRequest request )
    {
       .
       .
       .
        String alternatePomFile = null;
        if ( commandLine.hasOption( CLIManager.ALTERNATE_POM_FILE ) )
        {
            alternatePomFile = commandLine.getOptionValue( CLIManager.ALTERNATE_POM_FILE );
        }

        File userToolchainsFile;
        if ( commandLine.hasOption( CLIManager.ALTERNATE_USER_TOOLCHAINS ) )
        {
            userToolchainsFile = new File( commandLine.getOptionValue( CLIManager.ALTERNATE_USER_TOOLCHAINS ) );
            userToolchainsFile = resolveFile( userToolchainsFile, workingDirectory );
        }
        else
        {
            userToolchainsFile = MavenCli.DEFAULT_USER_TOOLCHAINS_FILE;
        }
...

This really looks like unnecessary code...cause it's not referenced anywhere else in this method...

[jira-importer]

Robert Scholte commented

It should be in sync with the way settings are handled. I can imagine that I missed something when introducing the global toolchains.

[jira-importer]

Karl Heinz Marbaise commented

That's not a problem...only those lines mentioned from #1465 to #1474 are simply not used anywhere else in this method and do not have a reference...so we can remove them...

[jira-importer]

Karl Heinz Marbaise commented

Stefan Eicher Would you like to offer a pull request ?

[jira-importer]

Stefan Eicher commented

ok I will do it tomorrow

[jira-importer]

Stefan Eicher commented

done -> #111

Grüsse Stefan

[jira-importer]

Karl Heinz Marbaise commented

Thanks Stefan for your support.
Done with 4c6d3a3462783507921d1a0100e3fa22ef8a98e4

[jira-importer]

Stefan Eicher commented

Hi Karl Heinz,

Your welcome. Thanks for your support. I just wanted to dig a bit deeper in maven and start reading the code. Thereby I thought if i do so, why not contribute even if it is just something that trivial. I was surprised by the attention I reveived. ... keeps me motivated.

Gruss Stefan