Skip to content

rework of installation guidelines#1103

Merged
fkiraly merged 17 commits intomainfrom
install-rework
Jul 16, 2021
Merged

rework of installation guidelines#1103
fkiraly merged 17 commits intomainfrom
install-rework

Conversation

@fkiraly
Copy link
Copy Markdown
Collaborator

@fkiraly fkiraly commented Jul 3, 2021

This is a rework of the installation guidelines.

They have grown a bit organically and were all over the place.

Was originally going to write the developer guidelines, but I found that these needed a cleaning first.

@fkiraly fkiraly requested a review from mloning as a code owner July 3, 2021 20:48
@fkiraly fkiraly requested a review from TonyBagnall July 3, 2021 20:48
@afzal442
Copy link
Copy Markdown
Contributor

afzal442 commented Jul 4, 2021

Looks cool now. Could we also add any instructions to install for WSL-2?

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jul 4, 2021

Looks cool now. Could we also add any instructions to install for WSL-2?

Oh, yes, of course! Sorry for forgetting. Let me take them from your PR, @afzal442.

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jul 4, 2021

@afzal442, rephrased it a bit and added it in two places: the prophet trouble shooting, and at the end of the "Windows" section. Have a look, and let me know whether you are happy with this.

@afzal442
Copy link
Copy Markdown
Contributor

afzal442 commented Jul 4, 2021

Looking better. Thanks!

@afzal442 afzal442 mentioned this pull request Jul 4, 2021
5 tasks
@fkiraly fkiraly linked an issue Jul 4, 2021 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@TonyBagnall TonyBagnall left a comment

Choose a reason for hiding this comment

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

I have not actually tried to install following these instructions, but the old ones clearly need updating. Couple of comments:
"thy can be installed" -> this can be installed
I'm not sure about this
"For brevity, we discuss steps 1 and 3 first; step 2 is discussed at the end, as it will depend on the operating system."
surely changing the order does not effect the overall brevity, and describing them out of order just seems strange,

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jul 5, 2021

"For brevity, we discuss steps 1 and 3 first; step 2 is discussed at the end, as it will depend on the operating system."
surely changing the order does not effect the overall brevity, and describing them out of order just seems strange,

Well, that's how it was, and it's perhaps not brevity but findability. If you have the massive "step 2" section in the middle with branches and sub-sections, you can't easily find the "step 3" section. Would it make sense to have "step 2" as a separate section, perhaps?

"thy can be installed" -> this can be installed

Do you have problems with my medieval English? Thine installatione shalt end bifore sundowne

@TonyBagnall
Copy link
Copy Markdown
Contributor

Well, that's how it was, and it's perhaps not brevity but findability. If you have the massive "step 2" section in the middle with branches and sub-sections, you can't easily find the "step 3" section. Would it make sense to have "step 2" as a separate section, perhaps?

maybe, I may also be being pedantic

Do you have problems with my medieval English? Thine installatione shalt end bifore sundowne

there must be a converter somewhere to refactor it all into olde English, its a good first issue

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jul 11, 2021

@TonyBagnall, I moved the build requirements into a new section - both the development version install and the advanced set-up reference it. The "step 2" is now in the right place, and references the section. This should address your point.

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jul 11, 2021

Points addressed, ready for review.

@fkiraly fkiraly requested a review from mloning July 11, 2021 12:00
@fkiraly fkiraly requested a review from TonyBagnall July 11, 2021 12:00
Copy link
Copy Markdown
Contributor

@mloning mloning left a comment

Choose a reason for hiding this comment

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

Two minor comments, everything else looks good

@fkiraly fkiraly requested a review from mloning July 15, 2021 21:17
@fkiraly fkiraly merged commit 88a60aa into main Jul 16, 2021
@fkiraly fkiraly deleted the install-rework branch July 16, 2021 18:16
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.

[DOC]Install requirements for WSL2 in installation guide

4 participants