search

readthedocs / readthedocs.org

The source code that powers readthedocs.org
https://readthedocs.org/
MIT License
8.03k stars 3.59k forks source link

The changelog contains non-merged PRs #3524

Open stsewd opened 6 years ago

stsewd commented 6 years ago

Details

Chagelog http://docs.readthedocs.io/en/latest/changelog.html#version-2-1-4

Not merged PRs:

How is the changelog generated?

stsewd commented 6 years ago

Also the previous release contains invalid issues as changelog: #3416, #3415

humitos commented 6 years ago

How is the changelog generated?

The changelog is generated with a script that is under a private repository. It uses this tool in the back, https://github.com/agjohnson/github-changelog

This is relatively new and we are testing it. Nice you have found some issues! This will help us to tweak it a little more and make it more robust.

agjohnson commented 6 years ago

My above fork should be easy to modify. I think eventually we want the search to only include pull requests that have been merged. The script currently searches for anything that has been closed, which is noisy. I've been hand editing the list of issues on release for now.

xrmx commented 6 years ago

Haven't looked at the code but the script should add only PR that has been merged to master branch, currently all the PR merged are listed.

stsewd commented 6 years ago

There is an option for that already https://github.com/agjohnson/github-changelog/blob/1915b5b24d1b0840eb15f5e07bcce0cd8c5e08a7/index.js#L31, we only need to modify how the script is called

stsewd commented 6 years ago

And we already pass that option https://github.com/rtfd/common/blob/ae892294342da555c90f69c6594277b0c546ede3/tasks.py#L89... @agjohnson how we really run the script?

xrmx commented 6 years ago

@stsewd that switch checks for issues with merged PR not merged to master branch.

stsewd commented 6 years ago

The current changelog contains PRs that just were closed too.

humitos commented 6 years ago

@stsewd the script is called via invoke prepare $RELEASE_VERSION and that prepare function is executed.

stsewd commented 6 years ago

I take a look at the chagelog repo, the logic seems correct, but it is using a deprecated package to pull the information https://www.npmjs.com/package/github, maybe it was a bug or the api was changed. So, maybe we want to look for an alternative or use the latest version of the original repo.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.