Promote the YAML config on project import #5444

davidfischer · 2019-03-13T04:59:25Z

This PR modifies the manual import screen to streamline things and promote the YAML config file for importing new repositories.

Link the YAML configuration docs
Update the YAML configuration docs to have a commented YAML file with the most common options
Remove the "advanced project options". All of those can be set after the project is built except documentation type (eg. sphinx) and that can be set in the YAML config.

Fixes #5414

Screenshots

Before

After

humitos

Looks good to me.

I think it's important to check corporate before merge it and probably adapt the code there as well.

This PR modifies the manual import screen to streamline things and promote the YAML config file for importing new repositories.

It seems that it does also affect the "automatic" import from VCS services, not only "Manual import".

humitos · 2019-03-13T08:56:47Z

readthedocs/projects/views/private.py

@@ -255,16 +254,11 @@ class ImportWizardView(ProjectSpamMixin, PrivateViewMixin, SessionWizardView):

    form_list = [
        ('basics', ProjectBasicsForm),
-        ('extra', ProjectExtraForm),


If we ending up with only one form in this list, we may remove the SessionWizardView and handle this as a normal view.

The corporate code actually relies on self.form_list. That's why I didn't completely refactor this view.

Why not refactor the corporate code for this as well?

I'm trying to make the scope of the change smaller rather than larger.

humitos · 2019-03-13T08:58:57Z

readthedocs/projects/views/private.py

-        kwargs['user'] = self.request.user
-        if step == 'basics':
-            kwargs['show_advanced'] = True
+        kwargs = {'user': self.request.user}


This ImportWizardView view is overridden in the corporate site, we probably need to make some changes there together with this PR and double check that this process still works there. Can you test it and apply the same changes there?

humitos · 2019-03-13T09:02:16Z

docs/config-file/v2.rst

@@ -8,15 +8,26 @@ Here is an example of how this file looks like:

 .. code:: yaml


Instead of the above line saying,

Here is an example of how this file looks like:

what about saying something like "Here is a minimum example configuration file:" or some better phrase than that one but making it clear that it's the minimum inviting the user to copy and paste as is other than keep reading the whole page.

I think there's a few changes so that the YAML docs are better "at a glance". I think that should be a separate PR but I can take care of this small change here.

davidfischer · 2019-03-13T15:29:01Z

It seems that it does also affect the "automatic" import from VCS services, not only "Manual import".

I'll take a look at that. I thought they were totally separate.

stsewd

I also think we should check the corporate site and that import from remote repos still works, but looks good to me :)

davidfischer · 2019-03-13T19:00:02Z

As-is, this change works fine with the corporate side and with the automatic import via VCS integrations. Interestingly, the automatic import with integrations sends the project description but that was never used with the advanced form.

This change does impact the corporate side in that there will be no way to specify before importing to use a different documentation type (eg. mkdocs, sphinx dirhtml) other than with the YAML file. Are we ok with that?

I do think that it could simplify things to refactor the ImportWizardView to not be a wizard but just be a regular view. That is a bigger change however as it involves completely refactoring the view on .org and where it is overridden in .com. I am not opposed to it but it will take a bit more time and coordination. Thoughts?

stsewd · 2019-03-13T21:14:58Z

This change does impact the corporate side in that there will be no way to specify before importing to use a different documentation type (eg. mkdocs, sphinx dirhtml) other than with the YAML file. Are we ok with that?

I'm not sure, I mean people using a different location from sphinx will face an error too or people that need a requirements file, and they need to go the admin page anyway.

So, I'm ok with removing the advanced options from the import menu, something we can't do is remove the advanced option from the admin page, not till we have a solution for changing settings to build old versions of docs

davidfischer · 2019-03-13T22:47:54Z

So, I'm ok with removing the advanced options from the import menu, something we can't do is remove the advanced option from the admin page, not till we have a solution for changing settings to build old versions of docs

Agreed.

humitos · 2019-03-18T09:27:03Z

This change does impact the corporate side in that there will be no way to specify before importing to use a different documentation type (eg. mkdocs, sphinx dirhtml) other than with the YAML file. Are we ok with that?

I'm not 100% convinced yet.

On one side, if we want to promote the YAML file and make the user to use it, giving the "Advanced" options at import time is not a good idea.

On the other side, the on-boarding experience for all the MkDocs projects won't be amazing (the first build will always fail --unless a YAML file propoerly configured already exists in the repo)

I do think that it could simplify things to refactor the ImportWizardView to not be a wizard but just be a regular view. That is a bigger change however as it involves completely refactoring the view on .org and where it is overridden in .com. I am not opposed to it but it will take a bit more time and coordination. Thoughts?

I'm 👍 on this. Although, if this PR is ready to be merged, we can merge it and make the refactor as a second step.

stsewd · 2019-03-18T15:39:57Z

On the other side, the on-boarding experience for all the MkDocs projects won't be amazing (the first build will always fail

Actually, there a lot of projects that will fail if they are not correctly setup, like having the correct requirements file, having the conf.py file in another directory than docs, needing to install the project, etc. We were trying to do some magic trying to guess this stuff, but in the end it caused more problems in my opinion.

ericholscher · 2019-03-18T15:47:46Z

I wonder if it's worth just adding the callout to the YAML file for now, but allowing users to edit advanced options still? That way we get both benefits, but don't make a worse experience for users who are not on YAML yet.

davidfischer · 2019-03-18T21:53:32Z

I just wanted to comment here in case we come back to this. We decided to go with #5485 which does the promotion without removing the advanced project settings for now. The advanced project settings can probably go away but it looks like it is a bigger change.

davidfischer added 2 commits March 12, 2019 21:35

Promote the YAML config on project import

c58c2e1

Remove unnecessary template

d833514

davidfischer requested review from stsewd and a team March 13, 2019 04:59

humitos reviewed Mar 13, 2019

View reviewed changes

stsewd reviewed Mar 13, 2019

View reviewed changes

davidfischer added 2 commits March 13, 2019 12:11

Small updates to the sample config file

a4e9f64

Return a dict literal

736a6e7

stsewd approved these changes Mar 13, 2019

View reviewed changes

davidfischer mentioned this pull request Mar 18, 2019

Promote the YAML config #5485

Merged

ericholscher closed this in #5485 Mar 18, 2019

stsewd deleted the davidfischer/promote-config-on-file-import branch March 18, 2019 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promote the YAML config on project import #5444

Promote the YAML config on project import #5444

davidfischer commented Mar 13, 2019

humitos left a comment

humitos Mar 13, 2019

davidfischer Mar 13, 2019

humitos Mar 13, 2019

davidfischer Mar 13, 2019

humitos Mar 13, 2019

humitos Mar 13, 2019

davidfischer Mar 13, 2019

davidfischer commented Mar 13, 2019

stsewd left a comment

davidfischer commented Mar 13, 2019

stsewd commented Mar 13, 2019

davidfischer commented Mar 13, 2019

humitos commented Mar 18, 2019

stsewd commented Mar 18, 2019

ericholscher commented Mar 18, 2019

davidfischer commented Mar 18, 2019

		@@ -8,15 +8,26 @@ Here is an example of how this file looks like:

		.. code:: yaml

Promote the YAML config on project import #5444

Promote the YAML config on project import #5444

Conversation

davidfischer commented Mar 13, 2019

Screenshots

Before

After

humitos left a comment

Choose a reason for hiding this comment

humitos Mar 13, 2019

Choose a reason for hiding this comment

davidfischer Mar 13, 2019

Choose a reason for hiding this comment

humitos Mar 13, 2019

Choose a reason for hiding this comment

davidfischer Mar 13, 2019

Choose a reason for hiding this comment

humitos Mar 13, 2019

Choose a reason for hiding this comment

humitos Mar 13, 2019

Choose a reason for hiding this comment

davidfischer Mar 13, 2019

Choose a reason for hiding this comment

davidfischer commented Mar 13, 2019

stsewd left a comment

Choose a reason for hiding this comment

davidfischer commented Mar 13, 2019

stsewd commented Mar 13, 2019

davidfischer commented Mar 13, 2019

humitos commented Mar 18, 2019

stsewd commented Mar 18, 2019

ericholscher commented Mar 18, 2019

davidfischer commented Mar 18, 2019