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

DOC fix link to devguide triaging guide #25035

Merged
merged 5 commits into from
Nov 25, 2022

Conversation

jasonjg
Copy link
Contributor

@jasonjg jasonjg commented Nov 25, 2022

Reference Issues/PRs

Towards #25024

What does this implement/fix? Explain your changes.

The link is working in a browser. Added the link to linkcheck_ignore.

Any other comments?

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.

Thanks for the pull request @jasonjg

doc/conf.py Outdated
@@ -630,6 +630,7 @@ def setup(app):
"https://microsoft.com/",
"https://www.jstor.org/stable/2984099",
"https://stat.uw.edu/sites/default/files/files/reports/2000/tr371.pdf",
"https://devguide.python.org/triage/triaging/index.html#becoming-a-member-of-the-python-triage-team",
Copy link
Member

Choose a reason for hiding this comment

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

the link works because there's a redirect. We should fix the docs to put the new link, which would be: https://devguide.python.org/triage/triaging/index.html

Copy link
Member

Choose a reason for hiding this comment

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

I think the issue here is actually the anchor #becoming-a-member-of-the-python-triage-team I have updated the issue with more info about each "broken" link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will do now. I found that was the case with the Google Open source link. My commit for that particular link will be to the corresponding .rst (about.rst)

Copy link
Member

Choose a reason for hiding this comment

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

Not sure what to do here and which section it was pointing to and what we wanted to say with this link.

I agree with @adrinjalali that we should try to find an appropriate link in this case rather than add it to the ignore list.

@@ -36,7 +36,7 @@ crucial to improve the communication in the project and limit the crowding
of the issue tracker.

Similarly to what has been decided in the `python project
<https://devguide.python.org/triaging/#becoming-a-member-of-the-python-triage-team>`_,
<https://devguide.python.org/triage/triaging/index.html>`_,
Copy link
Member

Choose a reason for hiding this comment

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

this sounds good to me, but the conf.py change then need to be reverted :)

Copy link
Member

Choose a reason for hiding this comment

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

Actually I think this may be the link we want (the anchor does still exist but in a different page somehow): https://devguide.python.org/triage/triage-team/#becoming-a-member-of-the-python-triage-team

Copy link
Contributor Author

@jasonjg jasonjg Nov 25, 2022

Choose a reason for hiding this comment

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

Missed in error. Working on that now.

Copy link
Contributor Author

@jasonjg jasonjg Nov 25, 2022

Choose a reason for hiding this comment

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

I removed the Devguide link from conf.py. I'll go back in and update the governance.rst link to the one you provided @lesteve

Copy link
Member

Choose a reason for hiding this comment

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

@jasonjg you might have missed this comment: #25035 (comment)

Copy link
Contributor Author

@jasonjg jasonjg Nov 25, 2022

Choose a reason for hiding this comment

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

https://devguide.python.org/triage/triage-team/#becoming-a-member-of-the-python-triage-team

Just checking before I make a commit. @lesteve Is the URL I quoted the one you want to use?

Copy link
Member

@lesteve lesteve Nov 25, 2022

Choose a reason for hiding this comment

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

Yes please 🙏.

I think this was the original intent of the link to point towards the Python Triage team policy (rather than instructions on how to triage an issue)

@lesteve lesteve merged commit f493dd3 into scikit-learn:main Nov 25, 2022
@lesteve
Copy link
Member

lesteve commented Nov 25, 2022

Merging, thanks a lot!

@jasonjg
Copy link
Contributor Author

jasonjg commented Nov 25, 2022

Thank you both for your assistance!

@jasonjg jasonjg deleted the jasonjg-fix-links branch November 25, 2022 17:17
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.

None yet

3 participants