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

DOC: Merge together DISTUTILS.rst.txt#template-files" and distutils.r… #13175

Closed
wants to merge 10 commits into from

Conversation

aashuli
Copy link
Contributor

@aashuli aashuli commented Mar 21, 2019

Documentation for distutil.rst.txt template file is merged with distutil.rst conversion-of-src-files .
The 2 sections describing similar distutils features are unified in the DISTUTILS.rst.txt file and discarded from distutils.rst file.

@eric-wieser
Copy link
Member

@mattip: what's your opinion on the direction we should merge these in? My feeling is that very few people consume the .txt files now, and we should lean towards putting things with the rest of the docs.

@mattip
Copy link
Member

mattip commented Mar 21, 2019

I don't have a strong opinion either way. At some point we could convert all the txt files to rst. I agree they are all consumed as html/pdf and not as unrendered documents.

For reference, here is the txt.rst and here the rst after this commit. Both documents need a good makeover, but the second is marginally worse than the first, so it makes sense to move the templating instructions to the first.

With that, the templating description is still split between the two documents, so some thought still needs to be given to more unification.

@aashuli
Copy link
Contributor Author

aashuli commented Mar 22, 2019

@mattip Can you suggest me what all further changes are to be done in this documentation.

@mattip
Copy link
Member

mattip commented Mar 22, 2019

@aashuli all the code from line 215 down in doc/source/reference/distutils.rst should be refactored into DISTUTILS.txt.rst, and replaced by a See ... for more information kind of link. Some thought should be given to the level of detail, perhaps the text you remove is a bit verbose, and would be better explained with examples rather than a language spec.

@charris
Copy link
Member

charris commented Mar 27, 2019

we should lean towards putting things with the rest of the docs

I agree, it only to make it easier to maintain by keeping things in fewer places.

@aashuli
Copy link
Contributor Author

aashuli commented Mar 29, 2019

@mattip I am sorry I was out of town for some personal reasons so could not finish this task yet and I just saw that @ojaswi12 is also working on this issue. So can I continue working on this?

@mattip
Copy link
Member

mattip commented Mar 29, 2019

#xref #13129, #13211

@mattip
Copy link
Member

mattip commented Mar 29, 2019

hmm. This wasn't xrefed this previously, and I didn't notice this PR when @ojaswi12 submitted the other one.

@aashuli
Copy link
Contributor Author

aashuli commented Mar 29, 2019

@mattip What more changes should i do to complete this task?

@mattip
Copy link
Member

mattip commented Mar 29, 2019

@aashuli in order not to harm your or @ojaswi12 's Outreachy efforts, perhaps you both could push forward, we will merge whichever seems better. You both may cite your efforts on your Outreachy application. In the future please be sure to xref your efforts to the issue, especially if you will only be contributing sporadically.

As for what remains to be done: the two sections still reside in separate documents, the information in the Conversion of .src files section of doc/source/reference/distutils.rst should be completely and coherently merged with the information you began to add to in doc/DISTUTILS.rst.txt, and the first should have a link to the second.

@aashuli
Copy link
Contributor Author

aashuli commented Mar 30, 2019

Thank you @mattip and sure I will remember this for next time.
I have merged the description in conversion of .src files section into DISTUTILS.rst.txt file as I found it to be suitable in the Users guide and I have linked it in Distutils.rst file.
Please review so that I can make the changes if required.

doc/DISTUTILS.rst.txt Outdated Show resolved Hide resolved
@mattip
Copy link
Member

mattip commented Mar 30, 2019

Note the red X next to your last commit. That means the build failed. You should open it and figure out what went wrong. Building the documents before pushing could save you some time here, then you could view them on your machine to verify everything builds and looks OK.

@aashuli
Copy link
Contributor Author

aashuli commented Apr 1, 2019

@mattip I am figuring out about that 1 unsuccessful check but can you please review the changes I have made.

@aashuli
Copy link
Contributor Author

aashuli commented Apr 1, 2019

@mattip I have resolved the problem of build failed and made the required changes. Please review

@mattip
Copy link
Member

mattip commented Apr 1, 2019

LGTM. One more nit about the link.

@aashuli
Copy link
Contributor Author

aashuli commented Apr 2, 2019

@mattip I couldn't understand why 1 check is pending yet. Is there anything to be done from my side?
Also today is the last day to fill the final application on Outreachy. So if my task is completed can I record this as a contribution for my application?

@mattip
Copy link
Member

mattip commented Apr 2, 2019

can I record this as a contribution for my application

Yes, please do. There is one more fix needed in the link you added, but please reference this anyway.

1 check is pending yet

Travis CI is having issues. It is fine to ignore that for now.

@mattip
Copy link
Member

mattip commented Jun 7, 2019

Replaced with #13729 to get rid of the extra commits due to a bad rebase

@mattip mattip closed this Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants