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

add how do i install oshinko cli article #244

Merged
merged 3 commits into from
Aug 23, 2018

Conversation

elmiko
Copy link
Collaborator

@elmiko elmiko commented Aug 22, 2018

No description provided.

@elmiko
Copy link
Collaborator Author

elmiko commented Aug 22, 2018

@emmurphy1 something light to start with =)

@elmiko elmiko requested a review from ruivieira August 22, 2018 21:01
:page-layout: howdoi
:page-menu_entry: How do I?

The Oshinko command line interface(CLI) tool is a binary application that gives you
Copy link
Contributor

Choose a reason for hiding this comment

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

space between 'interface' and '(CLI)'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

got it

//
// <List assemblies here, each on a new line>
[id='install-oshinko-cli']
= install the Oshinko command line interface tool
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if how to install oshinko cli should be the first how-to that people see

Copy link
Collaborator Author

@elmiko elmiko Aug 22, 2018

Choose a reason for hiding this comment

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

++

we have an open issue (#218) that speaks to a need for ordering on these things. i haven't come across a good solution to the problem.

i think some sort of categorized sections would be good, but i'm not sure what those categories should be. this is complicated by articles that straddle 2+ boundaries. totally open to suggestions here.

Copy link
Contributor

@rebeccaSimmonds19 rebeccaSimmonds19 left a comment

Choose a reason for hiding this comment

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

agree with @erikerlandson and the rest lgtm

Copy link

@emmurphy1 emmurphy1 left a comment

Choose a reason for hiding this comment

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

This is great, Mike. You have the concept of the procedure module pretty well. I just made a few editorial comments while I was there.

//
// <List assemblies here, each on a new line>
[id='install-oshinko-cli']
= install the Oshinko command line interface tool

Choose a reason for hiding this comment

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

Init cap: Install...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is actually a small artifact of our current "How do I" article format. we basically append the title to "How do I ....", i think it might be possible to programmatically fix this inline. i'll do some research.

:page-layout: howdoi
:page-menu_entry: How do I?

The Oshinko command line interface(CLI) tool is a binary application that gives you

Choose a reason for hiding this comment

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

interface (CLI) (add a space)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

++


.Prerequisites

* A Linux or MacOSX command terminal

Choose a reason for hiding this comment

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

This is fine, but our group has adopted the convention of using passive sentences for rereqs. So this would be:
A Linux or MacOSX command terminal is available.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

++

Copy link
Member

Choose a reason for hiding this comment

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

Just a nitpicking comment, the name officially changed from MacOSX to macOS :)

from the link:https://github.com/radanalyticsio/oshinko-cli/releases[project Releases page].

. Extract the `oshinko` binary from the archive to a location that is in your
terminal's `PATH`.

Choose a reason for hiding this comment

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

Not sure why path is formatted this way. Maybe ..in your terminal's path, for example... (Or maybe that doesn't make sense?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so, i put "PATH" in the block quotes because i am referencing the environment variable of the same name, the standard convention for adding something to the path. maybe that is too specific though, i will switch it.

changes
* add proper spacing for parenthesis
* update pre-reqs to use passive voice
* change MacOSX -> macOS
* remove code markup around "PATH"
Copy link
Member

@ruivieira ruivieira left a comment

Choose a reason for hiding this comment

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

@elmiko lgtm

@erikerlandson
Copy link
Contributor

ship it

This change is to help the modular template format look better on our
generated pages. The "title" blocks are created as div's in asciidoc and
these need some styling. This change will make them appear like h2
elements in our pages.
@elmiko elmiko merged commit 64667dc into radanalyticsio:master Aug 23, 2018
@elmiko elmiko deleted the add-oshinko-cli-install-howdoi branch August 23, 2018 14:36
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