-
Notifications
You must be signed in to change notification settings - Fork 34
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
Allow format issue output text within individual sections and in general #103
base: main
Are you sure you want to change the base?
Allow format issue output text within individual sections and in general #103
Conversation
Even if a custom format allows the links to be omitted, having a flag that controls the behavior when using the default format is a useful convenience that I think should be kept. |
@wilkinsona Could you describe the scenario of how Another option would be to use Thank you for your feedback, please say me if you have any other ideas |
I would only apply When using a custom format, I don't think |
Hi again @wilkinsona. I've put |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
import static io.spring.githubchangeloggenerator.ApplicationProperties.FormatPlaceholder.*; | ||
import static io.spring.githubchangeloggenerator.ApplicationProperties.FormatPlaceholder.TITLE; | ||
|
||
import io.spring.githubchangeloggenerator.github.service.Repository; |
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.
Please don't re-order the imports. It adds noise to the diff, making the changes harder to review. If you run ./gradlew build
, you will see failures from both Checkstyle and Spring Java Format that should be corrected please.
@DefaultValue("false") boolean addSections) { | ||
|
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.
This looks like a whitespace-only change. Please revert.
/** | ||
* Whether to generate a link to each issue in the changelog. | ||
*/ | ||
private final boolean generateLinks; |
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.
Why do we need a second property for this? The Issues
inner-class already has one.
public Issues(IssueSort sort, IssuesExclude exclude, Set<PortedIssue> ports, | ||
@DefaultValue("true") boolean generateLinks) { | ||
@DefaultValue("true") Boolean generateLinks, String defaultFormat) { |
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.
Why has generatedLinks
been changed from boolean
to Boolean
?
import static io.spring.githubchangeloggenerator.ApplicationProperties.FormatPlaceholder.NUMBER; | ||
import static io.spring.githubchangeloggenerator.ApplicationProperties.FormatPlaceholder.TITLE; | ||
import static io.spring.githubchangeloggenerator.ApplicationProperties.FormatPlaceholder.URL; | ||
|
||
import io.spring.githubchangeloggenerator.ApplicationProperties.ExternalLink; | ||
import io.spring.githubchangeloggenerator.ApplicationProperties.IssueSort; | ||
import io.spring.githubchangeloggenerator.ApplicationProperties.PortedIssue; | ||
import io.spring.githubchangeloggenerator.github.payload.Issue; | ||
import io.spring.githubchangeloggenerator.github.payload.Label; | ||
import io.spring.githubchangeloggenerator.github.payload.User; | ||
import io.spring.githubchangeloggenerator.github.service.GitHubService; | ||
import io.spring.githubchangeloggenerator.github.service.Repository; |
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.
Please correct the imports as per the problems reported by Checkstyle when you run ./gradlew build
.
Map<ChangelogSection, List<Issue>> sectionIssues) { | ||
sectionIssues.forEach((section, issues) -> { | ||
sort(section.getSort(), issues); | ||
content.append((!content.isEmpty()) ? String.format("%n") : ""); |
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.
Switching to isEmpty()
is unrelated to allowing more control over the formatting. Please revert.
Hi everyone! Here is a simple solution for issue #47