-
Notifications
You must be signed in to change notification settings - Fork 844
docs(oauth): use a variable "base_dir" home path for file stores #1760
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
Conversation
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.
📬 A note on change for reviewers!
|
||
rm -rf docs/reference | ||
|
||
pdoc slack_sdk --html -o docs/reference |
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.
🗣️ note: This change keeps this script consistent going forward! As far as I can tell, overriding the HOME
variable causes no problem for pdoc
or reference.
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.
Praise 🙏
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.
praise: Very clever!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1760 +/- ##
=======================================
Coverage 85.15% 85.15%
=======================================
Files 115 115
Lines 13068 13068
=======================================
Hits 11128 11128
Misses 1940 1940 ☔ View full report in Codecov by Sentry. |
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.
Clever solution 💯 🚀
|
||
rm -rf docs/reference | ||
|
||
pdoc slack_sdk --html -o docs/reference |
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.
Praise 🙏
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.
✅ Great solution @zimeg! I did a little digging and didn't find a configuration option, so you're solution was a great alternative! 🧠
|
||
rm -rf docs/reference | ||
|
||
pdoc slack_sdk --html -o docs/reference |
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.
praise: Very clever!
1503cb8
to
6337ddd
Compare
@WilliamBergamin @mwbrooks Thanks both for the kind reviews! 👾 ✨ I had to change the base branch after rushing to put this PR up, but the same changes remains so I will merge this now 📚 |
Summary
This PR replaces the
base_dir
default shown in documentation to use the "$HOME" placeholder instead of a particular path. Fixes #1752.Testing
No changes to functionalities! The docs I hope are more clear with this change too 📚
Notes
Category
/docs
(Documents)Requirements
python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh
after making the changes.