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

[BUG FIX] Enable alabaster render Travis CI badges for .com #147

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

webknjaz
Copy link

@webknjaz webknjaz commented Dec 17, 2018

Fixes #145

UPD: If somebody is looking to monkey-patch
it in their project while this isn't
merged β€” just apply this πŸ‘‡

                sanitizers/octomachinery@e5756d9

@bitprophet
Copy link
Collaborator

bitprophet commented Dec 18, 2018

Super appreciate you taking the initiative on this! πŸ‘ Immediate feedback:

  • I'm not sure I like the "auto" option, even though I am sure it took most of your time to get it working! Argh.
    • I totally get why it's tempting, but my inner sysadmin cringes at the idea of adding an additional HTTP request cycle to every doc build, with all that it entails: extra time/latency, tying ability to get docs built to another service's availability, etc. Yes, even though it's opt-in.
    • It's also a nontrivial addition of potential bug surface area, and I can speak from experience there. This is gonna have or grow bugs and that's more bug reports. I'm skeptical that the convenience is worth the cost, over time.
    • Minor vs the above two, but: users lacking requests (they exist! 😱) will have this blow up on them (not at import time, because you did guard against that, but when they actually flip their setting to auto). Documenting it could help, but either way it's still friction.
  • I'd be way more comfortable with travis_tld being a simple "set to com or org", which if I squint, is already in here...
    • maybe include the dot, too – I feel like the average user "thinks in" TLDs as being "dot-whatever"? so travis_tld = ".com". Maybe this is a better spot for magic/convenience: just do .lstrip('.') in case the user did supply it. EDIT: er, right, except without the context stuff we're left to Jinja. Pretty sure it has strip filters though.
  • Our own docs need to mention the new option! ✍️

@webknjaz

This comment has been minimized.

@webknjaz

This comment has been minimized.

webknjaz added a commit to sanitizers/octomachinery that referenced this pull request Mar 17, 2019
Also patch alabaster theme to work around the bug which should be fixed
by merging sphinx-doc/alabaster#147.
@webknjaz
Copy link
Author

webknjaz commented Jul 7, 2019

If somebody is looking to monkey-patch it in their project β€” just apply this πŸ‘‡

                sanitizers/octomachinery@e5756d9

@webknjaz
Copy link
Author

@bitprophet bump ⏰

@CraftSpider
Copy link

Any update on this? Travis started allowing automatic migration a while ago

ajyoon added a commit to ajyoon/bml that referenced this pull request Sep 14, 2021
this is a workaround since the alabaster theme doesn't support
the new travis domain

this would no longer be necessary with
sphinx-doc/alabaster#147
@ajyoon
Copy link

ajyoon commented Sep 14, 2021

any chance on getting this merged soon? as of June 15, 2021, the travis-ci.org site is essentially dead, so the existing Travis badge feature can't even work for holdouts.

@bitprophet
Copy link
Collaborator

At this point since org is dead I'd rather just change the existing code to s/org/com/ - is there any reason not to do so? I haven't dug into the rest of what this does though - if it adds additional value on top, then maybe just slimming this one down so it doesn't do the "tld setting" stuff would be fine.

@bitprophet bitprophet added this to the 0.8 milestone Oct 1, 2021
@webknjaz
Copy link
Author

webknjaz commented Oct 1, 2021

Dunno, I don't think I use the theme anymore, or Travis for that matter. It should be fine to just replace with .com tho.
OTOH it's probably more important to add support for the GHA badges these days.

@bitprophet bitprophet deleted the branch sphinx-doc:master January 13, 2023 00:42
@bitprophet bitprophet closed this Jan 13, 2023
@AA-Turner AA-Turner reopened this Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Option to specify the Travis platform
5 participants