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

Update readme #535

Merged
merged 1 commit into from
Feb 23, 2016
Merged

Update readme #535

merged 1 commit into from
Feb 23, 2016

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Jan 26, 2016

  • Removes mentions of install for fetch
  • New year, new copyright update
  • Cleans up some of descriptions
  • Remove grammatical errors

```
cd atomicapp
export PYTHONPATH=`pwd`:$PYTHONPATH
alias atomicapp="python `pwd`/atomicapp/cli/main.py"
Copy link
Member Author

Choose a reason for hiding this comment

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

i don't think this should be included ^^. let python devs do their own way of development

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 mind having it. Sometimes I use this and I always come back here as a reference.

@cdrage
Copy link
Member Author

cdrage commented Feb 8, 2016

pingarino @goern @vpavlin since you two did most of the initial README work according to git logs :)

@cdrage
Copy link
Member Author

cdrage commented Feb 9, 2016

Ping @dustymabe @rtnpro for review as this part is old stuff ^^ we don't use install anymore.

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

Just a call to Docker to package up the application and tag the resulting image.
Use 'docker build' to package up the application and tags the resulting image.

### Install and Run
Copy link
Contributor

Choose a reason for hiding this comment

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

Install --> Fetch ??

@dustymabe
Copy link
Contributor

Other than comments it looks good to me.

@cdrage
Copy link
Member Author

cdrage commented Feb 16, 2016

@dustymabe updated with your comments.

@@ -36,36 +38,35 @@ export PYTHONPATH=`pwd`:$PYTHONPATH
alias atomicapp="python `pwd`/atomicapp/cli/main.py"
```

### Build
### Build an Atomic App
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we be more clear about what this is doing? "Build an Atomic App" sounds like I am doing an application build (i.e. helloapache) vs packaging up the atomicapp software. Maybe this would be better: Build an Atomic App Container Image - for Container Execution. And then mention running it via Atomic CLI?

And maybe we even want to change the name of the previous section to Install Atomic App - for Local Execution

@dustymabe
Copy link
Contributor

@cdrage - can we get this updated?

@cdrage
Copy link
Member Author

cdrage commented Feb 22, 2016

Hey @dustymabe
So I updated it with the new title as well as the spelling error.

In regards to atomic cli execution. Should we add a separate section for alternatively using the atomic CLI?

Otherwise, rebased again ^^

@dustymabe
Copy link
Contributor

Build an Atomic App for containerized execution still sounds wrong.

What we are doing is containerizing the Atomic App software so it can then be run in a container but it isn't actually an "Atomic App". Building for containerized execution would be a better title. We should probably update Install this project to Install Atomic App Software Locally or something along those lines.

I say just digest these comments and update it based on your best judgement and then MERGE!!

@cdrage
Copy link
Member Author

cdrage commented Feb 23, 2016

@dustymabe
Updated based on your comments!

So I'm going to push this PR for now and I'll be adding a separate section for "atomic cli" execution this week. Work on doc updating for our upcoming release :)

cdrage added a commit that referenced this pull request Feb 23, 2016
@cdrage cdrage merged commit b8f890c into projectatomic:master Feb 23, 2016
@surajssd
Copy link
Collaborator

I think work on #543 can continue :)

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

Successfully merging this pull request may close these issues.

3 participants