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

Clarify the runtime directory location in installation docs #6624

Merged
merged 11 commits into from
Jun 13, 2023
Merged

Clarify the runtime directory location in installation docs #6624

merged 11 commits into from
Jun 13, 2023

Conversation

kazimir-malevich
Copy link
Contributor

book/src/install.md Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis added the A-documentation Area: Documentation improvements label Apr 6, 2023
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

The HELIX_RUNTIME and symlink options both allow you to install the Helix repo anywhere. Giving instructions that use ~/src explicitly makes it seem like it must be installed there which isn't the case and I think different people like to put their source code in different places.

So I think the runtime instructions for linux should just be more explicit about "the directory where the Helix repository is cloned." Mentioning ~/src as an example is ok but I think we should avoid hard-coding it in the instructions.

book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

The repository_directory placeholder looks like a regular directory name to me and I think that makes this more confusing. Instead I think we should keep the instructions mostly as-is and focus on the confusing parts of the HELIX_RUNTIME and ln steps: replacing the example absolute path and ensuring that we have cdd into the clone's directory.

book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
@pascalkuthe pascalkuthe added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2023
@David-Else
Copy link
Contributor

Please see #6809 , I have not addressed any of the things mentioned here on purpose, but please rebase when/if my PR gets merged as it changes some things around.

@kazimir-malevich
Copy link
Contributor Author

As of time speaking #6809 is still Open. I have attempted to synthesise directions from both @David-Else and @the-mikedavis.

book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

#6809 has been merged. Could you rebase or merge master to put this on top of those changes?

book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
book/src/install.md Outdated Show resolved Hide resolved
@kazimir-malevich
Copy link
Contributor Author

@David-Else Do I need to do anything more regarding this PR?

book/src/install.md Outdated Show resolved Hide resolved
@kazimir-malevich
Copy link
Contributor Author

kazimir-malevich commented Jun 5, 2023

I know I didn't create a local branch as I thought this would be so trivial, but we should be good to go now.

@the-mikedavis the-mikedavis changed the title runtime config made clearer Clarify the runtime directory location in installation docs Jun 13, 2023
@the-mikedavis the-mikedavis removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 13, 2023
@the-mikedavis the-mikedavis added the S-waiting-on-review Status: Awaiting review from a maintainer. label Jun 13, 2023
@pascalkuthe pascalkuthe merged commit 37fcd16 into helix-editor:master Jun 13, 2023
rozaliev pushed a commit to rozaliev/helix that referenced this pull request Jun 13, 2023
…itor#6624)

* runtime config made clearer

* following Unix FHS

* we probably want to install Helix as a regular user without sudo access

* suggestions adopted from @the-mikedavis

* attempted to synthesise comments given

* capitalisation of second header

* required changes hopefully made

* we should have a match now

* Linux windows dir match
Triton171 pushed a commit to Triton171/helix that referenced this pull request Jun 18, 2023
…itor#6624)

* runtime config made clearer

* following Unix FHS

* we probably want to install Helix as a regular user without sudo access

* suggestions adopted from @the-mikedavis

* attempted to synthesise comments given

* capitalisation of second header

* required changes hopefully made

* we should have a match now

* Linux windows dir match
wes-adams pushed a commit to wes-adams/helix that referenced this pull request Jul 4, 2023
…itor#6624)

* runtime config made clearer

* following Unix FHS

* we probably want to install Helix as a regular user without sudo access

* suggestions adopted from @the-mikedavis

* attempted to synthesise comments given

* capitalisation of second header

* required changes hopefully made

* we should have a match now

* Linux windows dir match
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
…itor#6624)

* runtime config made clearer

* following Unix FHS

* we probably want to install Helix as a regular user without sudo access

* suggestions adopted from @the-mikedavis

* attempted to synthesise comments given

* capitalisation of second header

* required changes hopefully made

* we should have a match now

* Linux windows dir match
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
…itor#6624)

* runtime config made clearer

* following Unix FHS

* we probably want to install Helix as a regular user without sudo access

* suggestions adopted from @the-mikedavis

* attempted to synthesise comments given

* capitalisation of second header

* required changes hopefully made

* we should have a match now

* Linux windows dir match
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Documentation improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants