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

Post about new configuration file #59

Merged
merged 14 commits into from
Feb 20, 2019

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jan 28, 2019

I wasn't sure what to include, hope this first draft makes sense, feel free to point me any problem with the language/grammar/typos, etc D:

Should I included more technical details? Like we moved this module from a separated repo, and we did a lot of changes in the rtd code?

Should I include that this project was part of my internship in rtd?

@ericholscher
Copy link
Member

Should I include that this project was part of my internship in rtd?

Yes!

I'd also include many more examples. I think now you're just stating benefits, but you should be showing people the benefits. Having some code examples of the YAML file would be a big win, especially around the neater stuff it can do.

Should I included more technical details? Like we moved this module from a separated repo, and we did a lot of changes in the rtd code?

I don't think this is important. Too "in the weeds".

@stsewd
Copy link
Member Author

stsewd commented Jan 29, 2019

I've added some examples, and mention the internship (not sure if I should add more details here?). Also, I've mentioned some new stuff that we are planning (actually not sure if redirects will make it readthedocs/readthedocs.org#4221).

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.

This is well written and explains the reasoning behind it. I made a few suggestions but I like this a lot. It's relatively short and sweet.

configuration-file-v2.rst Outdated Show resolved Hide resolved
but the main advantages of using a configuration file over the web interface are:

- Some settings are only available using a configuration file
- The settings are per version rather than per project.
Copy link
Contributor

Choose a reason for hiding this comment

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

Firstly, talking about the advantages of the config file is fantastic 💯!

However, I'd like to see some more details of some of these advantages. That could go lower down in the body or it could go right here. Some of them like "settings live in version control" are pretty straight forward, but settings being per version rather than per project requires a bit of explanation. For example, this would allow different versions of docs to be built with different versions of Python or different requirements.

configuration-file-v2.rst Outdated Show resolved Hide resolved
configuration-file-v2.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.

Great post. I think it needs to be fleshed out some more -- it reads like a summary or changelog instead of an announcement of the features. In general, this tells a lot of what but not a lot of why. I think explaining the why's will be really useful.


- Some settings are only available using a configuration file.

We are moving away from using the web interface.
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be a list item?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was a nested list, but it doesn't render correctly :/


- The settings are per version rather than per project.

This would allow different versions of docs to be built with different versions of Python
Copy link
Member

Choose a reason for hiding this comment

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

It feels weird to break up the list items like this, not sure what is trying to be done with it.

What did we change in the new version?
-------------------------------------

- All valid options from the web interface are available.
Copy link
Member

Choose a reason for hiding this comment

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

This should probably have some introduction that isn't in bullet form. Having an entire section that is just bullets feels overly brief.

-------------------

- Pipfile support
- Show the current configuration used in each build.
Copy link
Member

Choose a reason for hiding this comment

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

Again, it would be good to give more context here. A list item doesn't explain anything to the user. Each of these list items should instead be paragraphs, explaining the goal, why it would be good, and perhaps linking to an issue about it.


- Pipfile support
- Show the current configuration used in each build.
- Redirects per version of your docs.
Copy link
Member

@ericholscher ericholscher Feb 8, 2019

Choose a reason for hiding this comment

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

An example of how this might be expanded:

Historically we've only supported redirects at the Project level, because we didn't have version level configuration. With the addition of our YAML file, we can now support per version redirects. One use case for this is when you migrate to a new file structure in your upcoming release, but want all your old documentation links to continue working. The work around this issue is being tacked in issue #xxx.

@stsewd
Copy link
Member Author

stsewd commented Feb 15, 2019

I've expanded the post, I'm replacing the nested lists for paragraphs

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.

This is a great post. Just noted a few small changes.

configuration-file-v2.rst Outdated Show resolved Hide resolved
configuration-file-v2.rst Outdated Show resolved Hide resolved
configuration-file-v2.rst Show resolved Hide resolved
@ericholscher ericholscher merged commit 612e321 into readthedocs:master Feb 20, 2019
@ericholscher
Copy link
Member

👍 💯 🎉

@stsewd stsewd deleted the post-config-file branch February 20, 2019 15:40
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.

3 participants