Fix unlabeled, mixed labels, and unmapped labels handling #618

hunner · 2018-01-27T00:35:09Z

Issues/PRs filtered by --exclude-labels/--include-labels should never be added to sections.
Issues/PRs with labels mapping to multiple sections should map to default sections order of breaking, then enhancement, then fix.
Issues/PRs with labels mapping to multiple sections should map to default sections then the first-matching section in --add-sections.
Issues/PRs with labels mapping to multiple sections should map to the first-matching section in --configure-sections.
Issues/PRs with some mapped labels and some unmapped labels should respect the mapping rather than behaving like unmapped issues.
Issues/PRs with only unmapped labels should be added to the --merge-prefix/--issue-prefix section.
Issues/PRs with no labels should be added to the --merge-prefix/--issue-prefix section unless --add-pr-wo-labels/--add-issues-wo-labels is false.
Filter both PRs and issues equally.

olleolleolle · 2018-01-27T06:43:34Z

Good work! Fine analysis and useful in-code “why comments” in tricky places.

Thank you for bringing this to the project!

I’ll watch this closely.

eputnam

\o/

eputnam · 2018-02-08T00:47:24Z

lib/github_changelog_generator/generator/entry.rb

      github_site = @options[:github_site] || "https://github.com"
      project_url = "#{github_site}/#{@options[:user]}/#{@options[:project]}"

-      set_sections_and_maps
+      create_sections


is it worth changing this method name to generate_sections? it would match better with everything around it.

Reading the code, my personal take is “create_” reads easier.

We could move to that naming instead? But to me, that’s not a merge blocker for this change.

Thanks for keeping sharp eyes on This developing PR, @eputnam!

This method is about filling the @sections array with the Section.new objects to be filled out later on. I tried to make any generate_* methods be ones that return generated string content for the changelog.

eputnam · 2018-02-08T00:51:17Z

lib/github_changelog_generator/generator/entry.rb

-      body += merged_section_to_log(pull_requests) if @options[:pulls] && @options[:add_pr_wo_labels]
-      body
+      sort_into_sections(pull_requests, issues)
+      @sections.map(&:generate_content).join


i just learned what Array.map(&:foo) does. slick!

hunner · 2018-02-08T22:36:41Z

I think this is ready to go. It'd be nice if there were rubocop rules to catch missing @param and @return documentation.

olleolleolle · 2018-02-09T05:35:29Z

There is a little gem called yard-junk you can run to check your work. It has that goal. The output it makes is more helpful in finding wrong docblocks, though. Perhaps not the exact tool you asked for.

…

On 8 Feb 2018, at 23:36, Hunter Haugen ***@***.***> wrote: I think this is ready to go. It'd be nice if there were rubocop rules to catch missing @param and @return documentation. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

hunner · 2018-02-09T18:35:47Z

Thanks! I'll check it out.

olleolleolle · 2018-02-20T14:06:14Z

@hunner If there are any YARD improvements to be made, they're not a blocker to merging, if you feel this is ready for merge.

- Issues/PRs filtered by --exclude-labels/--include-labels should never be added to sections. - Issues/PRs with labels mapping to multiple sections should map to default sections order of breaking, then enhancement, then fix. - Issues/PRs with labels mapping to multiple sections should map to default sections then the first-matching section in --add-sections. - Issues/PRs with labels mapping to multiple sections should map to the first-matching section in --configure-sections. - Issues/PRs with some mapped labels and some unmapped labels should respect the mapping rather than behaving like unmapped issues. - Issues/PRs with only unmapped labels should be added to the --merge-prefix/--issue-prefix section. - Issues/PRs with no labels should be added to the --merge-prefix/--issue-prefix section unless --add-pr-wo-labels/--add-issues-wo-labels is false. - Filter both PRs and issues equally.

hunner · 2018-02-20T19:17:49Z

I added a few yard fixes pointed out by yard-junk just now, though not all of them were from these changes.

I'm not sure this is an enhancement. The command line arguments affected (--add-pr-wo-labels/--add-issues-wo-labels/--configure-sections/--merge-prefix/--issue-prefix/--exclude-labels/--include-labels) all existed before this change and had mostly-defined behavior, though some of the edge-cases were not defined or poorly handled. This PR is trying to bugfix around the edge cases of unlabeled/unmapped PRs and make the behavior more consistent with the various combinations of CLI arguments that were added in other recent PRs.

This change could be compared to v1.14.3 since 1.15.0 hasn't been finalized yet. It seems that #530 (--breaking-label/--breaking-labels) & #587 (--configure-sections/--add-sections) are features and this PR rectifies the behavior between the new features and the pre-existing ones.

You could could call this a feature if the handling of unlabeled PRs and PRs with unmapped labels is the feature that this implements, perhaps? Though without adding any arguments to do so it's arguable that this behavior was just undefined and is now being defined (which is a bugfix).

hunner · 2018-02-23T16:31:50Z

One change that I see this PR makes is that if --include-labels is specified, then only issues with those labels are included. Previous behavior was that issues with those labels plus unlabeled issues were included.

Since --add-issues-wo-labels and --add-pr-wo-labels default to true, perhaps those should control the unlabeled issue behavior and --include-label behavior should not affect unlabeled issue behavior.

That behavior should be configured by --add-issues-wo-labels and --add-pr-wo-labels

olleolleolle · 2018-02-24T09:23:06Z

I feel this is ready to merge to master. @hunner made a clear case for why. @hunner - any other opinion?

hunner · 2018-02-26T15:53:03Z

I think this is good to go!

olleolleolle

LGTM!

olleolleolle · 2018-02-26T16:03:23Z

@hunner Thank you!

hunner force-pushed the unlabeled_unmapped branch from 1ca4fa4 to aa3744c Compare January 30, 2018 01:29

hunner force-pushed the unlabeled_unmapped branch from d0cb917 to 9fe947b Compare February 8, 2018 00:44

eputnam reviewed Feb 8, 2018

View reviewed changes

hunner force-pushed the unlabeled_unmapped branch from 9fe947b to 91157bb Compare February 8, 2018 22:34

hunner changed the title ~~(WIP) Fix unlabeled, mixed labels, and unmapped labels handling~~ Fix unlabeled, mixed labels, and unmapped labels handling Feb 8, 2018

olleolleolle added bug enhancement and removed bug labels Feb 20, 2018

hunner force-pushed the unlabeled_unmapped branch from 91157bb to 1c4ef5e Compare February 20, 2018 17:09

Don't let --include-labels exclude unlabeled issues

69ca16b

That behavior should be configured by --add-issues-wo-labels and --add-pr-wo-labels

hunner force-pushed the unlabeled_unmapped branch from 0a0d975 to 69ca16b Compare February 23, 2018 18:58

olleolleolle approved these changes Feb 26, 2018

View reviewed changes

olleolleolle merged commit 466ea3e into github-changelog-generator:master Feb 26, 2018

hunner deleted the unlabeled_unmapped branch February 26, 2018 20:34

hunner mentioned this pull request Apr 12, 2018

Release prep 2.0.0 #641

Closed

skywinder added this to the 2.0.0 milestone Apr 15, 2018

This was referenced Jan 2, 2024

Cannot exclude PRs without labels. #292

Closed

Can't exclude issues without labels #302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unlabeled, mixed labels, and unmapped labels handling #618

Fix unlabeled, mixed labels, and unmapped labels handling #618

hunner commented Jan 27, 2018 •

edited

Loading

olleolleolle commented Jan 27, 2018

eputnam left a comment

eputnam Feb 8, 2018

olleolleolle Feb 8, 2018

hunner Feb 8, 2018

eputnam Feb 8, 2018

hunner commented Feb 8, 2018

olleolleolle commented Feb 9, 2018 via email

hunner commented Feb 9, 2018

olleolleolle commented Feb 20, 2018

hunner commented Feb 20, 2018

hunner commented Feb 23, 2018

olleolleolle commented Feb 24, 2018

hunner commented Feb 26, 2018

olleolleolle left a comment

olleolleolle commented Feb 26, 2018

Fix unlabeled, mixed labels, and unmapped labels handling #618

Fix unlabeled, mixed labels, and unmapped labels handling #618

Conversation

hunner commented Jan 27, 2018 • edited Loading

olleolleolle commented Jan 27, 2018

eputnam left a comment

Choose a reason for hiding this comment

eputnam Feb 8, 2018

Choose a reason for hiding this comment

olleolleolle Feb 8, 2018

Choose a reason for hiding this comment

hunner Feb 8, 2018

Choose a reason for hiding this comment

eputnam Feb 8, 2018

Choose a reason for hiding this comment

hunner commented Feb 8, 2018

olleolleolle commented Feb 9, 2018 via email

hunner commented Feb 9, 2018

olleolleolle commented Feb 20, 2018

hunner commented Feb 20, 2018

hunner commented Feb 23, 2018

olleolleolle commented Feb 24, 2018

hunner commented Feb 26, 2018

olleolleolle left a comment

Choose a reason for hiding this comment

olleolleolle commented Feb 26, 2018

hunner commented Jan 27, 2018 •

edited

Loading