Skip to content

fixes #224 add maven and gradle wrapper and make the script executable#249

Merged
stevehu merged 2 commits intodevelopfrom
issue224
Mar 15, 2019
Merged

fixes #224 add maven and gradle wrapper and make the script executable#249
stevehu merged 2 commits intodevelopfrom
issue224

Conversation

@stevehu
Copy link
Copy Markdown
Contributor

@stevehu stevehu commented Mar 10, 2019

No description provided.

@stevehu
Copy link
Copy Markdown
Contributor Author

stevehu commented Mar 10, 2019

Resolve #224

Copy link
Copy Markdown
Contributor

@243826 243826 left a comment

Choose a reason for hiding this comment

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

are identical files checked into multiple locations? would be nice if they are kept in one place and referred to from multiple locations.

mavenWrapperPropertyFileInputStream.close();
}
} catch (IOException e) {
// Ignore ...
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not declare main as throwing IOException and not swallow this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@243826 To be honest, I didn't even look into the wrapper class as I copied it from the spring boot generator. As @ddobrin mentioned below, we probably need to upgrade to the last version of maven wrapper.

System.out.println("Done");
System.exit(0);
} catch (Throwable e) {
System.out.println("- Error downloading");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could clean up this file by letting the exception escape.

<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<version.light-4j>1.5.18</version.light-4j>
<version.light-hybrid-4j>1.5.18</version.light-hybrid-4j>
<version.light-4j>1.5.32</version.light-4j>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not for this review but food for though - since 1.5.32 is not released, this should probably be 1.5.32-SNAPSHOT and should be uploaded to snapshot repository.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is always a good idea to have a snapshot version published and @NicholasAzar mentioned that before. There is still a lot of details we need to learn. Like where the snapshot version going to be published? How is maven going to figure out which version of the snapshot to be downloaded? What needs to be changed in the light-bot to support snapshot etc.

Copy link
Copy Markdown
Contributor

@ddobrin ddobrin left a comment

Choose a reason for hiding this comment

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

Should we consider setting the default to:
https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.3/,
up from 0.4.2?

@stevehu
Copy link
Copy Markdown
Contributor Author

stevehu commented Mar 11, 2019

@ddobrin Thanks for the link. I will try the latest version.

@stevehu stevehu merged commit 347c146 into develop Mar 15, 2019
@stevehu stevehu deleted the issue224 branch March 15, 2019 22:16
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.

3 participants