Skip to content

Updated the Quick-start guide#191

Merged
sushma-4 merged 17 commits intoapptainer:masterfrom
sushma-4:master
Jun 20, 2019
Merged

Updated the Quick-start guide#191
sushma-4 merged 17 commits intoapptainer:masterfrom
sushma-4:master

Conversation

@sushma-4
Copy link
Copy Markdown
Contributor

@sushma-4 sushma-4 commented Jun 10, 2019

Description of the Pull Request (PR):

Updated the quick guide so that the installation is described in the simplest way possible - compiling outside of GO etc. Also changed some command syntax supported by newer versions, updated the hyperlinks and interlinks to different sections of the docs.

This fixes or addresses the following GitHub issues:

Copy link
Copy Markdown

@jchavezlavalle jchavezlavalle left a comment

Choose a reason for hiding this comment

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

left some comments around @sushma-4 thanks!

Copy link
Copy Markdown
Contributor

@WestleyK WestleyK 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, couple suggestions/questions...

sushma-4 and others added 3 commits June 12, 2019 02:36
Co-Authored-By: WestleyK <westley@sylabs.io>
Co-Authored-By: WestleyK <westley@sylabs.io>
Copy link
Copy Markdown
Contributor

@GodloveD GodloveD left a comment

Choose a reason for hiding this comment

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

Thanks for these changes @sushma-4. I agree with most of them, but there are others I think should be reverted. I think the wget command should remain as a more efficient way to download singularity. Also I really don't think we want to replace internal links with external links. If we do so we will need to update all of the external links every time we update the docs versions or all of our links will be pointing to older versions of the docs. Better to just fix the internal references if they are busted.

Copy link
Copy Markdown
Contributor

@WestleyK WestleyK 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 to me 👍

@WestleyK WestleyK requested a review from GwendolynK-zz June 18, 2019 21:22
Copy link
Copy Markdown
Contributor

@GodloveD GodloveD left a comment

Choose a reason for hiding this comment

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

I still think the internal links should not be referencing an external URI. See my comments inline for details.

building from source) you must remove all of these files and directories to
For example, in a standard installation of Singularity 3.0.1 and beyond (when
building from source) you must remove all of these files and directories to
completely remove Singularity.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
completely remove Singularity.

This exact phrase was used in the sentence before. There isn't a need to repeat. One other note, to remain grammatically correct when removing this section of the sentence, we also need to remove the 'to' that proceeds it on the line right before.
End sentence should be "For example, in a standard installation of Singularity 3.0.1 and beyond (when building from source) you must remove all of these files and directories."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @GwendolynK
Does this sound better?

In a standard installation of Singularity 3.0.1 and beyond (when
building from source), the command sudo make install lists all the files as
they are installed. You must remove all of these files and directories to
completely remove Singularity.

Singularity.
.. note::
This is only a short manual for Quick Installation. For details on
different methods of installation, versions, and RPM etc, please refer to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
different methods of installation, versions, and RPM etc, please refer to
different methods of installation, versions, RPMs, etc, please refer to

@WestleyK is this still accurate technically? It is grammatically now...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm referring to https://sylabs.io/guides/3.2/user-guide/installation.html#build-and-install-an-rpm on our installation page. It should be just RPM IMO

sushma-4 and others added 6 commits June 19, 2019 16:52
Co-Authored-By: GwendolynK <43799653+GwendolynK@users.noreply.github.com>
Co-Authored-By: GwendolynK <43799653+GwendolynK@users.noreply.github.com>
Co-Authored-By: GwendolynK <43799653+GwendolynK@users.noreply.github.com>
Co-Authored-By: GwendolynK <43799653+GwendolynK@users.noreply.github.com>
Co-Authored-By: GwendolynK <43799653+GwendolynK@users.noreply.github.com>
@sushma-4 sushma-4 merged commit eb3142a into apptainer:master Jun 20, 2019
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.

5 participants