Skip to content
This repository has been archived by the owner on Jan 19, 2018. It is now read-only.

[EPIC] Major update to README and documentation #576

Merged
merged 1 commit into from
Mar 7, 2016

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Feb 25, 2016

Please be kind with commenting :), still a WIP. Going to be updating as I go. Just want to make this progress public.

It's HIGHLYYY recommended you view this through: https://github.com/cdrage/atomicapp/tree/update-readme-2 instead of looking at raw .md files. Gives you a better perspective on the new docs.

Left to do:

  • Guide on using atomicapp cli bare metal
  • Guide on using atomic cli on atomic host
  • List of all current features you can use in Nulecule file
  • Persistent storage doc

@bexelbie
Copy link
Contributor

If no one objects, I have put these changes in an etherpad: http://etherpad.osuosl.org/aad

This way we can easily make edits and notes. The files are all there with bold titles between them.

@cdrage
Copy link
Member Author

cdrage commented Feb 26, 2016

@bexelbie awesome, i'll work on it there / make changes / update the github as I go.

only problem is that unless it's synced with github we can't see the changes as to what it looks like on github itself. but this will be easier to edit and all :)

@bexelbie
Copy link
Contributor

Cool. My thought was that after 4-5 people had trawled through it, we would move back to using hte PR.

I may not get to it until Tuesday :(

@cdrage cdrage force-pushed the update-readme-2 branch 8 times, most recently from f50eece to 2c786ca Compare February 26, 2016 21:41

__Running Apache on Docker:__
```sh
▶ sudo atomicapp run --provider=docker projectatomic/helloapache
Copy link
Contributor

Choose a reason for hiding this comment

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

for these example commands let's make them compatible with the Atomic cli notation - I think with atomic cli we have to put the image name first and then the option after right?

so we should changes these examples to put the --provider= after the image name. Does that seem like the right thing to do?


__Graph__ is the most important section. In here we will indicate all the default parameters as well as all associated artifacts.

```yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add params: right below this line. so that the reader knows exactly where this text came from without having to fish for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks - and this is a test message so please ignore #dotests

Copy link
Contributor

Choose a reason for hiding this comment

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

One more test #dotests


Similarly, the kubernetes provider uses both `$image` and `$hostport` variables for pod deployment.

### ~/helloapache/answers.conf.gen
Copy link
Contributor

Choose a reason for hiding this comment

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

answers.conf.sample - gen is the file that gets generated on run.

@dustymabe
Copy link
Contributor

@cdrage - great job and thanks for taking an initiative on the docs! I look forward to the updates to fix the TODOs.

@jberkus
Copy link

jberkus commented Feb 27, 2016

The funny thing is that this line is the single most significant thing in the new readme:

For a list of already packaged examples, visit the [nulecule-library](https://github.com/projectatomic/nulecule-library) repo.

Previously we had a link to this repo: https://github.com/projectatomic/nulecule/tree/master/spec/examples/template

... which, of course, has no examples. I never knew about nulecule-library; we didn't link to it before.

More to come!

- OpenShift 3
- Marathon (Mesos)

Providers represent various deployment targets. They can be added by placing the artifact within the respective in `provider/` folder. For example, placing `deploy_pod.yml` within `providers/kubernetes/`.
Copy link

Choose a reason for hiding this comment

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

The examples in nulecule-library have the providers directly in the artifacts directory. Where is this provider/ directory supposed to be?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should update it to say artifacts like all of our examples use. In truth you can use any folder name, but here we should use ./artifacts.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's my bad, it's suppose to be artifacts >.>

@jberkus
Copy link

jberkus commented Feb 27, 2016

The startup guide should explain that, if you're running these on your desktop and not in the ADB or similar, you'll need to run a lot of the commands using "sudo", such as all of the "atomicapp" commands.

7. [File handling](docs/file_handling.md)
8. [Specification coverage](docs/spec_coverage.md)
9. [Contributing](CONTRIBUTING.md)
10. [Dependencies](docs/requirements.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know where these links fit in - they link to other files but there are also sections below for most of the same headings. Are the sections below supposed to be there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the best thing would be to keep it the way it is but move it somewhere else within this file. I see value I just don't think where these links currently sit is the best place in the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. I wanted to follow something along the lines of how docker and kubernetes do their documentation. i quite like that there is an index that shows direct links to the files I'd like to view.

having it near / at the top of the readme.md would be great.

You're right about the headers (some of them are duplicate headers in the readme that link to the same doc)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah right now it just looks confusing. I'm not saying we shouldn't have the links, but we should clear things up a bit.

@cdrage cdrage force-pushed the update-readme-2 branch from 663c8c6 to 8a030ea Compare March 3, 2016 20:45
@cdrage
Copy link
Member Author

cdrage commented Mar 3, 2016

I have updated based on your comments @dustymabe . The major difference being start_guide.md comments.

```
docker build -t [TAG] .
```
## Documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe say this:

This README contains some high level overview information on Atomic App. The detailed documentation for Atomic App resides in the docs directory. Links to each section have been conveniently listed below:

@dustymabe
Copy link
Contributor

ok @cdrage.. I had just one final comment. I'm good to go, although I know jberkus may have had some more stuff he wanted us to add.

either way we can keep adding to it in new PRs if we want to go ahead and get this in.

@jberkus
Copy link

jberkus commented Mar 4, 2016

I think this is fine for a first version. We'll have more, but then we'll always have more.

@cdrage cdrage force-pushed the update-readme-2 branch from 8a030ea to a4815ec Compare March 4, 2016 15:19
@cdrage
Copy link
Member Author

cdrage commented Mar 4, 2016

Hey @dustymabe

I've updated it again based on your comments (only inclusion is your comment about the index / links)

With the exception of a section on how to use the answers.conf file (I'll add this in a later PR / section), it's good for merging.

@dustymabe
Copy link
Contributor

cdrage merge away.

cdrage added a commit that referenced this pull request Mar 7, 2016
[EPIC] Major update to README and documentation
@cdrage cdrage merged commit ce03e52 into projectatomic:master Mar 7, 2016
@cdrage
Copy link
Member Author

cdrage commented Mar 7, 2016

✨ Wooooo! :) Thanks everyone for the docs reviews!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants