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 Fix rendered doc affected paths for towncrier fragments #30361

Merged

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Nov 28, 2024

Seen in #30360, the generated link is wrong and does not exist, it should point to the changelog file.
image

Copy link

github-actions bot commented Nov 28, 2024

✔️ Linting Passed

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

Generated for commit: 6bbce97. Link to the linter CI: here

@lesteve
Copy link
Member Author

lesteve commented Nov 28, 2024

It seems like this is working, see this PR affected paths

image

I am going to revert the changes that I made for testing.

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. Even more complexity added to this file but that's life.

I find a bit weird that a bug fix ends up in v1.7.rst in the check rendered doc even though it expected to be in 1.6.1 for instance. Kind of unrelated to this PR...

@lesteve
Copy link
Member Author

lesteve commented Nov 28, 2024

LGTM. Even more complexity added to this file but that's life.

Yep, I thought about what we said some time ago, next time we will put the logic in a Python file and I was like, hmmm yeah next time maybe I'll do it 😅

I find a bit weird that a bug fix ends up in v1.7.rst in the check rendered doc even though it expected to be in 1.6.1 for instance. Kind of unrelated to this PR...

Agreed it's a bit weird, at the same time what I care about in a PR is to check the HTML rendering is fine. I don't know if other projects (astropy, numpy, etc ...) have a better setup.

@jeremiedbb
Copy link
Member

All right let's merge to not delay things further

@jeremiedbb jeremiedbb merged commit 49199b5 into scikit-learn:main Dec 3, 2024
30 checks passed
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Dec 4, 2024
virchan pushed a commit to virchan/scikit-learn that referenced this pull request Dec 9, 2024
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.

2 participants