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

CI generate changelog in doc ci #30231

Merged
merged 5 commits into from
Nov 7, 2024

Conversation

glemaitre
Copy link
Member

closing #30220

Trying to generate the changelog in the documentation CI to be able to check the changelog generation when reviewing.

@glemaitre glemaitre changed the title Ci doc generate changelog CI generate changelog in doc ci Nov 6, 2024
Copy link

github-actions bot commented Nov 6, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: b09b250. Link to the linter CI: here

@@ -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"
Copy link
Member Author

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.

Copy link
Member

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

Copy link
Member Author

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.

@glemaitre
Copy link
Member Author

@ogrisel @adrinjalali @jeremiedbb This PR is now generating the changelog in main and in the pull-request. We don't do this process in non-PR (i.e. 1.6.X, etc.) because in this case, we will be running towncrier ourself during the release process and check if there is a need to manually amend it.

Copy link
Member

@adrinjalali adrinjalali left a 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"
Copy link
Member

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

Copy link
Member

@jeremiedbb jeremiedbb left a 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

Comment on lines +186 to +187
if [[ "$CIRCLE_BRANCH" =~ ^main$ || -n "$CI_PULL_REQUEST" ]]
then
Copy link
Member

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 ?

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

ah nice 👍

@adrinjalali adrinjalali merged commit 5810fd3 into scikit-learn:main Nov 7, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants