-
-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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
CI generate changelog in doc ci #30231
CI generate changelog in doc ci #30231
Conversation
@@ -262,7 +262,8 @@ package = "sklearn" # name of your package | |||
|
|||
[tool.towncrier] | |||
package = "sklearn" | |||
filename = "doc/whats_new/notes-towncrier.rst" | |||
filename = "doc/whats_new/v1.6.rst" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something that we discussed with @lesteve. Basically, we will need to bump this version at each major release for the next one. We can check later if we can automatized this but for the moment I think this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we should also make sure this step is included in the "how to release guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. My plan is to make an update during the upcoming release process.
@ogrisel @adrinjalali @jeremiedbb This PR is now generating the changelog in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the documentation nit, LGTM.
@@ -262,7 +262,8 @@ package = "sklearn" # name of your package | |||
|
|||
[tool.towncrier] | |||
package = "sklearn" | |||
filename = "doc/whats_new/notes-towncrier.rst" | |||
filename = "doc/whats_new/v1.6.rst" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we should also make sure this step is included in the "how to release guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The rendering looks good and the link on the home page is now working again. Just a minor comment
if [[ "$CIRCLE_BRANCH" =~ ^main$ || -n "$CI_PULL_REQUEST" ]] | ||
then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need it in main ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such that we have an changelog published on the /dev
website as well. I thought that it could be handy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah nice 👍
closing #30220
Trying to generate the changelog in the documentation CI to be able to check the changelog generation when reviewing.