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

Point users to commercial solution for their private repositories #5849

Merged
merged 6 commits into from
Jul 22, 2019

Conversation

Abhi-khandelwal
Copy link
Contributor

Fix #5770


{% blocktrans %}
Here only public repositories are listed, If you want to find your private
repositories, you can find on <a href="https://travis-ci.com/">trvais-ci.com</a>
Copy link
Member

Choose a reason for hiding this comment

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

The URL should point to readthedocs.com the commercial service of ReadtheDocs. Also we need to make sure the message does not appear on the commercial website.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohk sir , I will change the url from travis-ci.com to readthedocs.com and second thing , Can you please tell me that where can I access commercial website? in source code so that I can fix this.

Copy link
Member

Choose a reason for hiding this comment

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

The source code from the commercial site is private, you can add an named block like https://github.com/rtfd/readthedocs.org/blob/c0bcb243b14b465695cacf9aab644da23c411ca7/readthedocs/templates/projects/project_import.html#L216 so we can override that value in the other template

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohk thanks @stsewd

@@ -70,7 +70,10 @@
</span>

<h1>{% trans "Import a Repository" %}</h1>

{% block non-commercial %}
Copy link
Member

Choose a reason for hiding this comment

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

The translation block shouldn't be removed. And the block name could be something like import-tip or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I use? {% blocktrans import-public-repo %} ?

Copy link
Member

Choose a reason for hiding this comment

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

@Abhi-khandelwal
Copy link
Contributor Author

Hello @saadmk11 , Do I have to make more changes?

Copy link
Member

@saadmk11 saadmk11 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. 💯
Looks good. what do you think? @stsewd

<p>
{% blocktrans trimmed %}
Here only public repositories are listed, If you want to find your private
repositories, you can find on <a href="https://readthedocs.com/">readthedocs.com</a>.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
repositories, you can find on <a href="https://readthedocs.com/">readthedocs.com</a>.
repositories, you can find them on <a href="https://readthedocs.com/">readthedocs.com</a>.

@@ -70,7 +70,16 @@
</span>

<h1>{% trans "Import a Repository" %}</h1>

{% block import-repo %}
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 you should put your code inside

{% if has_connected_accounts %}
    .....
{% endif %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohk

Copy link
Contributor Author

@Abhi-khandelwal Abhi-khandelwal Jun 29, 2019

Choose a reason for hiding this comment

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

What message should I put in else condition ?

Copy link
Member

@saadmk11 saadmk11 Jun 29, 2019

Choose a reason for hiding this comment

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

just wrap your code with the condition.

{% if has_connected_accounts %}
    <your code>
{% endif %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @stsewd , Do I have to make more changes?

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.

This looks good. I think we can improve the block name before merging.

Also, I'd like @davidfischer to take a look at the copy of the message since he will probably have some suggestions on how to communicate this better.

@@ -70,7 +70,18 @@
</span>

<h1>{% trans "Import a Repository" %}</h1>

{% if has_connected_accounts %}
{% block import-repo %}
Copy link
Member

Choose a reason for hiding this comment

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

The block name could be a little more descriptive, like "private-repo-tip" or similar.

@davidfischer
Copy link
Contributor

How about something like:

We're only showing your public repositories. For private projects and many of features, please use <a href="#">Read the Docs for Business</a>.

Copy link
Contributor

@davidfischer davidfischer left a comment

Choose a reason for hiding this comment

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

Copy looks great!

Copy link
Member

@saadmk11 saadmk11 left a comment

Choose a reason for hiding this comment

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

Good Work Thanks 💯

@Abhi-khandelwal
Copy link
Contributor Author

Thank you @saadmk11

@Abhi-khandelwal
Copy link
Contributor Author

Abhi-khandelwal commented Jul 19, 2019

@humitos , Do I need to make some changes ?

@humitos
Copy link
Member

humitos commented Jul 22, 2019

No! This is great! Thanks for your collaboration!

@Abhi-khandelwal
Copy link
Contributor Author

No! This is great! Thanks for your collaboration!

your welcome :)

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.

Point users to commercial solution for private repositories
5 participants