-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
[tool-maven] Add blog release generator maven cmd #7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few details/questions (more notes to ensure it is not passing under your radar) but look good to merge
.build(); | ||
try (final Jsonb jsonb = JsonbBuilder.create(new JsonbConfig().setProperty("johnzon.skip-cdi", true))) { | ||
final var workDir = Files.createDirectories(Paths.get(workdir)); | ||
Files.deleteIfExists(Path.of(workDir + "/releases-blog-post.adoc")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it needed? if you open it without options it will overwrite it anyway at https://github.com/yupiik/tools-maven-plugin/pull/7/files#diff-7a9a6ae89337aab9648e9b123bfc69faa4c9f7f0fc9ee020b1856df90709e6e2R180 no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes because if you run it several time, it will append so we need to clean up at each run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, it does cause you asked to append, if you dont it does not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We append for each repository, so another option will be to write to the file at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change flow
var out = new StringBuilder(); | ||
|
||
out.append("== ").append(githubRepository.replaceAll("yupiik/", "")).append("\n\n"); | ||
DateFormat df = new SimpleDateFormat("dd/MM/yyyy"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datetimeformatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes, good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
throw new IllegalStateException(e); | ||
} | ||
}); | ||
try (final var outputStream = Files.newOutputStream(Path.of(workDir + "/releases-blog-post.adoc"), APPEND, CREATE)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files.writeString?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
2ea414b
to
ab151f2
Compare
No description provided.