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

Add minimal viable docs for addons #11444

Merged
merged 10 commits into from
Jul 9, 2024
Merged

Add minimal viable docs for addons #11444

merged 10 commits into from
Jul 9, 2024

Conversation

plaindocs
Copy link
Contributor

@plaindocs plaindocs commented Jul 1, 2024

Address #11425

This is how I'm planning to handle the list of Addons. Add minimal changes to existing docs (I'm expecting some addons to need more than this) and link to them centrally from /addons.

How do we feel about this?


📚 Documentation previews 📚

@plaindocs plaindocs self-assigned this Jul 1, 2024
@plaindocs
Copy link
Contributor Author

@ericholscher @humitos this is a pretty minimal start. If you're happy with the direction, I'll do a little more cleanup and we can merge this.

Are we considering the Sponsorship thing as an addon?

@humitos
Copy link
Member

humitos commented Jul 3, 2024

I've added this PR to my list. I will try to review it soon.

Are we considering the Sponsorship thing as an addon?

It's implemented as an addon. However, it doesn't really add too much value to the user, so I'm not sure how to talk about this one in particular. We have to mention it somehow, but maybe what we already have is enough since the user doesn't care about the implementation: https://docs.readthedocs.io/en/stable/advertising/ethical-advertising.html

@plaindocs
Copy link
Contributor Author

Yeah, that tracks. I'd suggest removing it from this 'Addons' list in that case.

@plaindocs
Copy link
Contributor Author

plaindocs commented Jul 3, 2024

@humitos when you take a look, keep an eye out for any places that need more detail in the difference between the new and old feature, like in search.

Also, the advertising section is pretty hidden in the RTD docs, I linked to Sponsorship instead, because that's what I found.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Overall, this is great! I'm happy that we are moving forward with this because we really need to have something at least.

I left some suggestions with ideas and questions as well. I think we are going in a great direction here 👍🏼

I'm sorry. I know all of this is confusing --we are in the middle of a transition to addons and new dashboard as well at the same time 🙃 and both are pretty big and important changes.

docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/analytics.rst Outdated Show resolved Hide resolved
docs/user/flyout-menu.rst Show resolved Hide resolved
docs/user/notifications.rst Outdated Show resolved Hide resolved
docs/user/flyout-menu.rst Show resolved Hide resolved
docs/user/server-side-search/index.rst Show resolved Hide resolved
docs/user/server-side-search/index.rst Outdated Show resolved Hide resolved
@plaindocs
Copy link
Contributor Author

Overall, this is great! I'm happy that we are moving forward with this because we really need to have something at least.

I left some suggestions with ideas and questions as well. I think we are going in a great direction here 👍🏼

I'm sorry. I know all of this is confusing --we are in the middle of a transition to addons and new dashboard as well at the same time 🙃 and both are pretty big and important changes.

This is great feedback, super helpful! I've fixed most of the small stuff, there are a few outstanding

@plaindocs plaindocs marked this pull request as ready for review July 4, 2024 13:43
@plaindocs plaindocs requested a review from a team as a code owner July 4, 2024 13:43
@plaindocs
Copy link
Contributor Author

Not done yet I think, but we're getting there.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Looks great.

I left some suggestions and some small changes about moving content from the new notifications page to the existing versions page.

Apart from that, I think this PR is ready to merge 👍🏼 . I'm pinging @ericholscher just in case he want to take a look at these changes as well; but I think we are ready to move forward after those changes.

docs/user/analytics.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/addons.rst Show resolved Hide resolved
docs/user/addons.rst Outdated Show resolved Hide resolved
docs/user/notifications.rst Outdated Show resolved Hide resolved
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Lots of great improvement here 💯

docs/user/addons.rst Show resolved Hide resolved
docs/user/flyout-menu.rst Show resolved Hide resolved
docs/user/guides/technical-docs-seo-guide.rst Outdated Show resolved Hide resolved
docs/user/notifications.rst Outdated Show resolved Hide resolved
docs/user/pull-requests.rst Outdated Show resolved Hide resolved
docs/user/pull-requests.rst Outdated Show resolved Hide resolved
@plaindocs
Copy link
Contributor Author

OK folks, I think I've addressed all feedback with this last commit.

@humitos one last 👀 and then :shipit: !

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Excellent! 💯

@plaindocs plaindocs enabled auto-merge (squash) July 9, 2024 12:20
@humitos
Copy link
Member

humitos commented Jul 9, 2024

It seems there is a checks error:

diff --git a/docs/user/addons.rst b/docs/user/addons.rst
index 8f8bd4def..dff978bf8 100644
--- a/docs/user/addons.rst
+++ b/docs/user/addons.rst
@@ -54,4 +54,4 @@ Individual configuration options for each addon are available in :guilabel:`Sett
 
 #. Click on a project name.
 #. Go to :guilabel:`Settings`, then in the left bar, go to :guilabel:`Addons (Beta)`.
-#. Configure each Addon individually.
\ No newline at end of file
+#. Configure each Addon individually.

Once that's fixed, it will auto merge. It's missing just a small newline 😄

@plaindocs plaindocs merged commit 6aaa330 into main Jul 9, 2024
4 of 5 checks passed
@plaindocs plaindocs deleted the sam/addons branch July 9, 2024 12:45
@plaindocs
Copy link
Contributor Author

🎉

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.

None yet

3 participants