-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 Notification for failure to add deploy key on manual imported project #11573
base: main
Are you sure you want to change the base?
Conversation
8c19795
to
9554248
Compare
We were unable to find this repository in your {{ provider_name }} account. | ||
If this is a private repository, you'll need to | ||
<a href="https://docs.readthedocs.io/page/guides/importing-private-repositories.html"> | ||
add the deploy key manually | ||
</a>. | ||
""" |
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.
I think we want to communicate this in a more direct way mentioning manual import or similar.
When adding a project manually, you need to add the deploy key manually. Follow this guide to add the deploy key manually.
ccing @agjohnson since I think he will have a better copy for this notification.
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.
That is closer to what I was thinking, however we should also cover public repositories with this notification. All manually created projects will still need to set up a webhook manually, and private repos will need a deploy key. The notification should probably more generically mention set up steps, the above is specific to deploy keys.
When adding a project manually, your repository will require extra configuration steps. Follow this guide to finish set up of your project.
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.
Don't we already mention the extra steps in the new dashboard?
That links to https://docs.readthedocs.io/en/stable/intro/add-project.html, which doesn't mention anything about SSH keys for private repos, we should mention that.
Anyway, I think it would be useful to show a notification after the project is imported, as a reminder to the user. But don't think we need to do that when adding an ssh key.
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.
We mention that extra steps are required beginning project creation, but still want some notification afterwards that these are problems the user must address.
I would not mention the GitHub account, as this is not required to even be connected, and I wouldn't mention deploy keys specifically, as the project/repo is missing webhooks at this point too. The notification should be inclusive to both, as both.
Eventually, this could be onboarding steps with individual actions required, but we're not quite there yet.
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.
Yeah, I mean, we need a notification, but doesn't look like that notification is the one from this PR, since this notification was originally created on the ssh key setup step (I'm no longer using this notification in the .com PR).
""" | ||
).strip(), | ||
), | ||
type=WARNING, |
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.
I think this would be best as an info message, instead of warning message
type=WARNING, | |
type=INFO, |
We were unable to find this repository in your {{ provider_name }} account. | ||
If this is a private repository, you'll need to | ||
<a href="https://docs.readthedocs.io/page/guides/importing-private-repositories.html"> | ||
add the deploy key manually | ||
</a>. | ||
""" |
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.
That is closer to what I was thinking, however we should also cover public repositories with this notification. All manually created projects will still need to set up a webhook manually, and private repos will need a deploy key. The notification should probably more generically mention set up steps, the above is specific to deploy keys.
When adding a project manually, your repository will require extra configuration steps. Follow this guide to finish set up of your project.
ref https://github.com/readthedocs/readthedocs-corporate/issues/1806