Skip to content
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

Fixed issue #304 - entries of previous tags are included. #438

Conversation

thorsteneckel
Copy link
Collaborator

This pull requests fixes the issue #304 .

ATTENTION: This pull request changes the current behavior!

Current behavior:
Use since_tag or due_tag and every issue, PR etc. that happened before the last tag will get added to the last tag.

New behavior:
Use since_tag or due_tag and every issue, PR etc. that happened in the timeframe of the tags is added to the tags. Previous elements are not added to the list.

Feedback is welcome. Cheers!

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.

Damn this is nice to have done.

How can we make this testable?

I want to express the behavior using English and RSpec.

@thorsteneckel
Copy link
Collaborator Author

thorsteneckel commented Nov 4, 2016

@olleolleolle I thought about adding tests and stuff but since there were none checking the current behavior I skipped them too. Imho there are some bigger changes on the structure necessary to test this properly. My suggestion would be to add a layer that makes it possible to mock github calls completely.

// edit:
This would come with the additional benefit of having a stable internal backend (Github client) API that makes it easier and safer to switch the gem from github_api to octokit or wherever.

@olleolleolle
Copy link
Collaborator

Does this change in behavior mean that options' descriptions need to change? Any other documentation around due_tag, since_tag?

@olleolleolle
Copy link
Collaborator

Update: I've now re-read the relevant issue that this fixes. It's great!

@olleolleolle olleolleolle merged commit bcacabc into github-changelog-generator:master Nov 4, 2016
@thorsteneckel
Copy link
Collaborator Author

Wow! thanks @olleolleolle for merging this that fast.

@thorsteneckel thorsteneckel deleted the feature-skip-prev-tags branch November 4, 2016 22:54
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.

2 participants