Skip to content

When tags are excluded, do not include those tags in diff links. #930

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 1, 2021
Merged

When tags are excluded, do not include those tags in diff links. #930

merged 2 commits into from
Feb 1, 2021

Conversation

ameir
Copy link
Contributor

@ameir ameir commented Jan 15, 2021

If I am excluding tags, I do not expect them to be referenced in any way. Instead, they are still referenced in the diff links. This changes behavior to make this so. This may be considered a breaking change, but the current behavior doesn't make sense to me as it is -- I'd be happy to hear counterpoints.

This fixes #842.

ccannell67
ccannell67 approved these changes Jan 15, 2021
View reviewed changes
Copy link

@ccannell67 ccannell67 left a comment

Choose a reason for hiding this comment

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

LGTM

olleolleolle
olleolleolle approved these changes Feb 1, 2021
View reviewed changes
Copy link
Collaborator

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Ah, this test takes some reading to grok. I just read it, and I agree with the change!

@olleolleolle olleolleolle merged commit b1d8af2 into github-changelog-generator:master Feb 1, 2021
@douglasmiller
Copy link
Contributor

@olleolleolle do you have any timeline for the 1.15.3 release that will include this change?

@olleolleolle
Copy link
Collaborator

I am maintaining a Release Draft. That includes a summary of what is added/changed/fixed, with links to each PR.

I now made an attempt to run the tool on itself, using the latest code, and I had a StackLevelTooDeep error with a stacktrace pointing at:

Found 75 tags
Fetching tags dates: 75/75
Sorting tags...
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:163: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:401: warning: The called method `iterate_pages' is defined here
Received issues: 659
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:187: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:401: warning: The called method `iterate_pages' is defined here
Pull Request count: 420
Filtered pull requests: 313
Filtered issues: 150
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:217: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:401: warning: The called method `iterate_pages' is defined here
Fetching events for issues and PR: 463
Fetching closed dates for issues: 463/463
Traceback (most recent call last):
	1362: from /Users/olle/.rvm/gems/ruby-2.7.1/gems/async-1.28.9/lib/async/task.rb:265:in `block in make_fiber'
	1361: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/generator/generator.rb:55:in `block in compound_changelog'
	1360: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/generator/generator.rb:152:in `fetch_issues_and_pr'
	1359: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/generator/generator_fetcher.rb:48:in `add_first_occurring_tag_to_prs'
	1358: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/generator/generator_fetcher.rb:65:in `associate_tagged_prs'
	1357: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:367:in `fetch_tag_shas'
	1356: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:367:in `each'
	1355: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:368:in `block in fetch_tag_shas'
	 ... 1350 levels...
	   4: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:353:in `commits_in_tag'
	   3: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:353:in `each'
	   2: from /Users/olle/opensource/github-changelog-generator/lib/github_changelog_generator/octo_fetcher.rb:354:in `block in commits_in_tag'
	   1: from /Users/olle/.rvm/gems/ruby-2.7.1/gems/sawyer-0.8.2/lib/sawyer/resource.rb:54:in `[]'
/Users/olle/.rvm/gems/ruby-2.7.1/gems/sawyer-0.8.2/lib/sawyer/resource.rb:56:in `rescue in []': stack level too deep (SystemStackError)

With that issue blocking me, I can't release yet.

@ameir
Copy link
Contributor Author

ameir commented Feb 18, 2021

@olleolleolle can you test if the generator works with this PR: #934 ? It appears that the async needs some tuning, but probably best to revert until it's better investigated.

@olleolleolle
Copy link
Collaborator

I will be printing the Async Task tree when I get around to it.

@douglasmiller
Copy link
Contributor

douglasmiller commented Feb 18, 2021 •
edited
Loading

It looks like the primary issue is that commits_in_tag is recursive without any tail call optimization. It just keeps calling itself until the stack is exhausted.

I'll takea deeper look in a few hours to see if it can be easily rectified.

@douglasmiller douglasmiller mentioned this pull request Feb 19, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olleolleolle olleolleolle olleolleolle approved these changes

@ccannell67 ccannell67 ccannell67 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Full Changelog" uses tags that are excluded
4 participants
@ameir @douglasmiller @olleolleolle @ccannell67