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

Guides: how to import a private project using an ssh key #8336

Merged
merged 5 commits into from Jul 14, 2021

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jul 12, 2021

This is mostly extracted from the private submodules guide,
and also includes steps for Azure DevOps.

Rendered version:

This is mostly extracted from the private submodules guide,
and also includes steps for Azure DevOps.
@stsewd stsewd requested a review from a team July 12, 2021 19:57
@astrojuanlu astrojuanlu self-requested a review July 13, 2021 09:04
Copy link
Contributor

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Followed the procedure without issues, LGTM 👍🏽

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

This guide is great! Noted that this might actually make better content for feature documentation rather than as a guide, where it isn't as discoverable.

docs/guides/importing-private-repositories.rst Outdated Show resolved Hide resolved

#. Manually import your project using an SSH URL
#. Allow access to your project using an SSH key
#. Setup a webhook to build your documentation on every commit
Copy link
Contributor

Choose a reason for hiding this comment

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

This block is really important to this doc, but it's easy to gloss over this. I'm not sure how to make these steps more prominent, but it should be really clear that RTD doesn't work if you forget the webhook set up.

Maybe a definition list would make this list appear a bit more definitive, or at least stand out more?

Copy link
Member Author

Choose a reason for hiding this comment

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

with a definition list, items are over-spaced, well, tried putting the number as the definition and the text as content. I have changed the items to be marked as bold instead, if you have other ideas how to use the def list let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find a good example of what I'm describing. The bold definitely helps, and I'm not sure if a definition list is necessarily the answer either.

What I've done is something in between a local TOC and definition list for this sort of scenario -- that is, lay out the steps and describe them just a little bit. The additional description on each step provides just a little more hand holding for the user, and it replaces the local TOC.

If you wanted to play around with it more, here is what I'm describing:

To accomplish the thing you need to do, there are a few steps that you will need
to take:

`The first step you'll take`_
    This is a very important step due because a, b, and c

`The next step`_
    You can't miss this step. This step is important because your project
    won't do the thing you need to do without it

`The final step`_
    Lastly, you definitely can't forget this step, because of other reasons

The first step you'll take
--------------------------

The next step
-------------

The final step
--------------

docs/guides/importing-private-repositories.rst Outdated Show resolved Hide resolved
docs/guides/private-submodules.rst Outdated Show resolved Hide resolved
docs/guides/private-submodules.rst Outdated Show resolved Hide resolved
docs/guides/private-submodules.rst Outdated Show resolved Hide resolved
stsewd and others added 2 commits July 13, 2021 17:22
Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Changes look great! Just noted small grammar updates

docs/guides/importing-private-repositories.rst Outdated Show resolved Hide resolved
docs/guides/importing-private-repositories.rst Outdated Show resolved Hide resolved
docs/guides/private-submodules.rst Outdated Show resolved Hide resolved
Co-authored-by: Anthony <aj@ohess.org>
@stsewd stsewd enabled auto-merge (squash) July 14, 2021 18:49
@stsewd stsewd merged commit bf5c688 into master Jul 14, 2021
@stsewd stsewd deleted the guide-manual-import branch July 14, 2021 18:55
Comment on lines +35 to +37
After importing your project the build will fail,
because Read the Docs doesn't have access to clone your repository.
To give access, you'll need to add your project's public SSH key to your VCS provider.
Copy link
Member

Choose a reason for hiding this comment

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

Regarding UX, it would be good to do something different here to avoid the initial build to fail:

  1. do not trigger the first build if the project is imported manually
  2. on manual imports, add an extra step that shows the just generated project's public SSH key to the user for them to copy and paste into their VCS service

I'd like if we can implement 2) on the new templates to make the onboarding process nicer. cc @agjohnson

Comment on lines +14 to +19
- You can manage which submodules Read the Docs should clone using a configuration file.
See :ref:`config-file/v2:submodules`.

.. note::

Make sure you are using ``SSH`` URLs for your submodules
(``git@github.com:readthedocs/readthedocs.org.git`` for example)
in your ``.gitmodules`` file, not ``http`` URLs.
- Make sure you are using ``SSH`` URLs for your submodules
(``git@github.com:readthedocs/readthedocs.org.git`` for example)
in your ``.gitmodules`` file, not ``http`` URLs.
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think having 2 different notes is more visible and easier to parse while reading.

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

4 participants