Skip to content

Exclude merge PRs from changelog with filter #77

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

Merged
merged 2 commits into from
Jun 22, 2021

Conversation

tlsvda
Copy link
Contributor

@tlsvda tlsvda commented Jun 17, 2021

No description provided.

@tlsvda tlsvda force-pushed the master_exclude-merge-prs-from-changelog branch 2 times, most recently from fe2e6d2 to 0da9dd0 Compare June 17, 2021 14:50
$style->listing($this->changelogGenerator->getUnreleasedChangelog()->getUnreleasedChanges());

$filteredChangeLogElements = $this->filterChangeLogElements(
$this->changelogGenerator->getUnreleasedChangelog()->getUnreleasedChanges()
Copy link
Contributor

Choose a reason for hiding this comment

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

Filtering the merge requests is not a responsibility of this class.

I feel this logic should be included inside the change log generator class that is being used.
That will also remove the need for the extra check in the markdownFormatter since that will only format anything that is inside the Changelog which is being returned from the ChangeLogGenerator.

Maybe even better would be if this was an optional configuration for our changelog generator, but lets not include that in the scope for now.

Copy link
Contributor Author

@tlsvda tlsvda Jun 17, 2021

Choose a reason for hiding this comment

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

You mean this file: PullRequestChangelogGenerator.php

This is already an optional configuration, this is only for PRs & Commit with specifically [MERGE] in the title

Copy link
Contributor

Choose a reason for hiding this comment

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

Its optional to use; Not an optional configuration.
As this is an open-source project, some users might have a workflow that includes merges. Or they would want to use a different tag to specify merge requests with.

Maybe this issue is only specific with our workflow and others don't even need it. Maybe they are doing trunk-based development?

Those kind of questions mean that this feature requires a little more thought; Now, or later.

@tlsvda tlsvda force-pushed the master_exclude-merge-prs-from-changelog branch 2 times, most recently from 1633b19 to 3240415 Compare June 18, 2021 06:54
@tlsvda tlsvda requested a review from VSlokker June 20, 2021 13:27
@tlsvda tlsvda force-pushed the master_exclude-merge-prs-from-changelog branch from 3240415 to 3512c52 Compare June 22, 2021 09:16
@tlsvda tlsvda requested review from christiaan and VSlokker June 22, 2021 09:25
@tlsvda tlsvda merged commit 9738012 into master Jun 22, 2021
@tlsvda tlsvda deleted the master_exclude-merge-prs-from-changelog branch June 22, 2021 09:59
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