Release prep 2.0.0 #641
Release prep 2.0.0 #641
Conversation
|
This is rebased onto #640 to get the changelog to generate correctly. |
| # | ||
| # @return [Array] filtered issues and pull requests | ||
| def filter_issues_for_tags(newer_tag, older_tag) | ||
| filtered_pull_requests = filter_by_tag(@pull_requests, newer_tag) | ||
| filtered_issues = delete_by_time(@issues, "closed_at", older_tag, newer_tag) | ||
| filtered_issues = delete_by_time(@issues, "actual_date", older_tag, newer_tag) |
There was a problem hiding this comment.
be careful with that. it should be tested well. I remember, that have a reason to change actual_date to closed_at before. Need to check logs to understand why. but there is couple of tricky cases related with that string
There was a problem hiding this comment.
Okay, I can do some more looking into it. If I recall, the actual_date was problematic for PRs but not for issues, but that should be irrelevant due to #619 using git history for PRs now.
Dump of investigation thoughts follow.
Before #504, PRs and issues were filtered by actual_date / actual_date. Those values are not from the github API but actually set by the fetcher in GCG based on GitHub API events:
The merged_at and closed_at values on PRs and issues do come from the GitHub API at the issue level rather than the event level.
For pull requests, the following is now irrelevant as #619 now finds PR/tag relationships based on git history, but it is helpful to have written down since I'm digging into it anyway.
For pull requests, the merged_at/closed_at (identical) time is often one second later than the date of the merge commit due to GitHub marking the PR closed one second after creating the merge commit. But GCG detects the tag time based on the tagged commit's commit time, which would place any PR whose merge commit was tagged one second later than the tag, thus not considered part of the tagged release even though it was. For this, it would be better to consider both the PR close time and the tag release time to be the merge event commit's time.
For pull requests, the merged_at/closed_at time could be more than one second different from the actual_date if a user uses hub merge <url> locally and pushes the merge commit later, but this is very rare.
For pull requests, if the events do not have a "merged" event to detect the merge commit then the merged_at time is used anyway.
For issues, the events are inspected for any event of "closed" and if so checked for a commit_id to use for actual_date. If the "closed" event has no commit_id then the closed_at time is used for actual_date and there's no difference.
For issues, if the "closed" event has a commit_id then the author commit time is used as the actual_date. This occurs when a commit message has the magic words in it and is somewhat rare. The milestone matching is probably much more useful. This would at least be improved if the issue was associated with a release based on its commit sha as PRs are now rather than time-based. I believe this also is better to use the commit time, for example this issue being closed a day after the commit time of the closing commit.
Issues just don't have a good mechanism for determining which release they go in, especially for projects with parallel branches. They should probably be determined in some order of priority like this:
- Milestones matching the tag.
- "Closing commit: " similar to the "Rebased commit: " comment in Use git history to find PRs in a tag instead of time #619 (this doesn't exist yet)
- closing event commit_id author time (or committer time? Hmm...)
- The
closed_atapi field.
Numbers 3 and 4 are what actual_date does do, whereas closed_at only does the last one.
There was a problem hiding this comment.
This dump of investigation notes - a lot of valuable work. Thank you.
(The last bullet list made me think of "data organizing" vs "rendering" again. If it were possible to have a preview of how the issues/PRs/tags/releases/milestones would be presented, there could be room for an interactive step, where the user could decide which Section a line should be in. If those choices were persisted and re-used, it could be neat.)
There was a problem hiding this comment.
Currently one of the biggest limitations of gcg for persisting information across changelog builds (also see #582) is that it does not depend on having access to any files in the git repository and works 100% based on data available via the github api. That puts a limit on a lot of ideas that would be neat, but would be a pretty big increase in the complexity to implement.
|
@olleolleolle About adding stash/githab/bitbucket support to the 2.0.0 milestone... it seems like these integrations are very large tasks and the code for them has not yet been started. It has been a long time since the last release of GCG, and there are many changes for the better ready to go. Perhaps we should push for a release to get the recent changes out the door, and then a regular release cadence again before tackling bigger integrations later? Also "github" is literally in the name of the gem :D CC #649 |
|
would love to see a new release with all the new changes |
|
Guys, come on! I`m really excited to use the new changes 🎉 |
|
Is there any way to install this gem directly from GitHub while we wait for the release? |
|
A way to install
Then, this command is possible:
|
| [Full Changelog](https://github.com/skywinder/github-changelog-generator/compare/v1.15.0.pre.beta...v1.15.0-rc) | ||
| ## [v2.0.0](https://github.com/skywinder/github-changelog-generator/tree/v2.0.0) (2018-04-12) | ||
|
|
||
| [Full Changelog](https://github.com/skywinder/github-changelog-generator/compare/v1.15.0.pre.rc...v2.0.0) |
There was a problem hiding this comment.
noticed all URLs still use "skywinder"; that something to be changed?
There was a problem hiding this comment.
Hello!
(And yes, we can change those. A re-generation of the document would do the right thing.)
|
Please please can we get a release? We've been waiting on this for months and want to roll this out to over 30 modules @olleolleolle @skywinder @thaJeztah |
|
The reason the code review has taken so long is that 2.0 includes a new way of figuring out what Issue and PR goes into which "release". The line with "actual_date" in it. I'm inclined to release a 2.0 and then iterate on that, until we're in a better place. |
|
@hunner Would you be able to run a CHANGELOG re-generation? |
|
This tool looks great, I'm waiting for the release before starting to use it in projects, and am especially looking forward to the Keep-a-Changelog labels from #636. Thank you! |
|
Looking forward to the new release. We had to drop the tool in the mean time. |
|
Any ideas when this might be released? I've been using this version myself and it's been working pretty well. It does seem to require I specify the username and repo for everything, which it used to pick up automatically, but otherwise works great 👍 |
|
@hunner @olleolleolle @skywinder since we are bumping, can we also think about #644 ? |
|
@skywinder Such a great tool :-D |
|
There have been 147 commits to master since the last release. Is 2.0 ready to release yet? I continue to use this version, just manually installed. Would love to see something released soon! Perfection is the enemy of progress, right? 😉 |
|
Is this still relevant? Shall we close it and revise the plans for the 2.0.0 milestone (and other, future ones)? @olleolleolle @skywinder |
|
We stalled. Perhaps we need to take things piecemeal, one step at a time. This PR can close, since we are fixing defects, still. We'll get there, though, we'll get there. Thanks for all the investigation work, @hunner! |
|
Closing for now :) |
|
Would we maybe be able to get some alpha releases as you work on this piecemeal? Would love to try out the new stuff! |
I know that a bunch of the PR/issue titles need to be changed an a lot need labels, so this PR is to see how the release looks at the moment.
I commented on #451 and #501 already, but I think those are considered breaking changes so a major bump may be needed. Also #530, #587, #618, #619, and #636 are somewhat significant changes so a major bump may be nice for calling those out too.