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

Optional Fix for README #101

Merged
merged 7 commits into from Jun 20, 2020
Merged

Optional Fix for README #101

merged 7 commits into from Jun 20, 2020

Conversation

miladsade96
Copy link
Contributor

Separating navigating to stack directory and Adding to PATH
Because some of users may navigate to stack directory first and and enter this command(export PATH="$PATH:pwd/stack-2.1.3-linux-x86_64-static/) in order to install stack command that causes error below in $PATH:

No such file or directory

@LKedward
Copy link
Member

@everythingfunctional This reminds me of a question I wanted to ask about the instructions for installing stack: why does the README recommend following the 'Manual Download' instructions as opposed to using the simpler install script or distribution package managers? I used the Ubuntu distribution package followed by running $ stack upgrade which was much simpler and works fine.

The instructions for installing stack are pleasantly comprehensive (and cross-platform!) and I think we should simply refer users to this page and allow them to choose their preferred installation method.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

I think this is a positive change, as our specific instruction was Linux only.

@miladsade96
Copy link
Contributor Author

I think this is a positive change, as our specific instruction was Linux only.

Thank you @milancurcic for approving this PR.

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

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

Thanks @everlookneversee , I agree this is an improvement.
Unless Brad had a specific reason for recommending the manual installation method, I think this is good to merge with one change: can you revert 7ee74b1?

@miladsade96
Copy link
Contributor Author

Thanks @everlookneversee , I agree this is an improvement.
Unless Brad had a specific reason for recommending the manual installation method, I think this is good to merge with one change: can you revert 7ee74b1?

Ok, I'll revert it.

@everythingfunctional
Copy link
Member

@certik wanted to support people who didn't have sudo access to install system wide. I agree that the official stack install instructions are generally the preferred way, so I support this change. We may want to have a separate section to put the manual install instructions somewhere just in case.

@certik
Copy link
Member

certik commented Jun 17, 2020

I would like to keep it in some form, as requiring a sudo access is a non-starter on almost any machine where I would like to use fpm (I don't have sudo access).

@jvdp1
Copy link
Member

jvdp1 commented Jun 17, 2020

I would like to keep it in some form, as requiring a sudo access is a non-starter on almost any machine where I would like to use fpm (I don't have sudo access).

I agree. Could this link just be mentioned to replace the deleted section. Furthermore, all OS are described there.

@certik
Copy link
Member

certik commented Jun 17, 2020

I think that link would work.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@miladsade96
Copy link
Contributor Author

@certik
What should we do for this PR?

Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@certik certik merged commit 63acb2f into fortran-lang:master Jun 20, 2020
certik added a commit to certik/fpm that referenced this pull request Jun 20, 2020
@miladsade96 miladsade96 deleted the Fix_ReadMe branch June 21, 2020 06:51
@miladsade96
Copy link
Contributor Author

@certik Thanks for merging.

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.

None yet

6 participants