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

mkcloud: enhance documentation #228

Merged

Conversation

MaximilianMeister
Copy link
Contributor

No description provided.

them are listed below. To get a complete list run: `./mkcloud -h`
`mkcloud` consists of different steps which can be run independently, provided the order is logical.
```
all -> expands to: cleanup prepare setupadmin addupdaterepo runupdate prepareinstcrowbar instcrowbar rebootcrowbar setupcompute instcompute proposal testsetup rebootcompute
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we want to duplicate all this expansion info, because it's too easy for it to get out of sync with the code. DRY! Better to ensure that the user can find out what any step expands to by running mkcloud in a particular way.

@aspiers
Copy link
Contributor

aspiers commented Feb 25, 2015

Thanks a lot for working on this! Needs a bit more discussion with others about where the docs should go.

@MaximilianMeister MaximilianMeister force-pushed the doc-enhancements branch 3 times, most recently from a5d817e to 68551b3 Compare February 26, 2015 08:06
@MaximilianMeister
Copy link
Contributor Author

I would vote for moving the quick introduction to the docs/mkcloud.md and further read the usage from that file

@MaximilianMeister MaximilianMeister force-pushed the doc-enhancements branch 2 times, most recently from ee87bf5 to e25ef8a Compare February 26, 2015 10:30
aspiers referenced this pull request in MaximilianMeister/automation-cloud Mar 2, 2015
@MaximilianMeister
Copy link
Contributor Author

I'm not sure we want to duplicate all this expansion info, because it's too easy for it to get out of sync with the code. DRY!

Do you mean DRY should apply to code + documentation together? I see documentation as a separate thing, its about finding out what is what (and what expands to what). Documentation always gets a bit out of sync. For here I think it is still manageable. If someone has a change, the docs should just be updated as well. Why make it any more complicated?

@aspiers
Copy link
Contributor

aspiers commented Mar 3, 2015

Do you mean DRY should apply to code + documentation together?

Yes.

I see documentation as a separate thing

It's separate, but it's also strongly coupled to code and does not justify duplication.

its about finding out what is what (and what expands to what)

The user can already see expansions from mkcloud -h.

Documentation always gets a bit out of sync.

I strongly disagree! That's exactly what we should try hard to avoid! It is totally unacceptable for documentation to get out of sync.

For here I think it is still manageable. If someone has a change, the docs should just be updated as well. Why make it any more complicated?

Because people will always forget to update both places. That is the reason at the heart of why DRY is such an important golden rule! If there is repetition, at least one of the duplicate copies will get out of sync and cause problems.

Another approach would to be to define the expansions in a separate config file, and then have the code reference that, and have the docs auto-generated from it via a simple script. Then there would be two copies of the expansion data, but that would be OK because only one of them would be authoritative. But that's probably overkill in this case. I'm just mentioning it as an alternative technique for sticking to DRY in other scenarios.

do
[[ $line =~ "\`mkcloud\` consists of" ]] && out=1
[[ $line =~ "\`\`\`" ]] && continue
[[ $line =~ "### Additional Information" ]] && exit 1
Copy link
Member

Choose a reason for hiding this comment

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

I guess you meant

[[ $line =~ "### Additional Information" ]] && out=0

@rsalevsky
Copy link
Contributor

I think we should split the documentation into different files. Maybe usage, configuration and installation? The advantage is we have a clear structure and we can reference explicit parts in the cmd-line help or in the GitHub readme.
A second thing what I would change is to put the options from the mkcloud part in a table to get a better overview/structure.

@aspiers
Copy link
Contributor

aspiers commented Mar 4, 2015

They sound like good suggestions to me. Main thing I'm concerned about is eliminating the risk of further divergence between docs and code in the future.

@jdsn
Copy link
Member

jdsn commented Mar 26, 2015

If our goal is to keep mkcloud as a big beast, then I would agree that it might make sense to have the documentation separately.

But what we currently try to achieve is, to split mklcoud into a set of smaller specialized tools. And these small tools should bring their own documentation with themselves. So the refactoring will also take care, that the documentation gets split off into smaller chunks.

Therefore 👎 from me.

@aspiers
Copy link
Contributor

aspiers commented May 4, 2015

@MaximilianMeister There are some valuable changes in this PR which we want to merge, but also some bits we disagreed with. Please could you drop the contentious bits so we can get the other fixes in?

@jdsn
Copy link
Member

jdsn commented May 4, 2015

Rebase needed as the two places of documentation inside mkcloud were merged meanwhile. And I am still no convinced that this is the way to go with depending on a different file for the usage.
It also makes it even harder when working on the script to quickly search for variable definitions and usages.

@MaximilianMeister
Copy link
Contributor Author

@jdsn there will be no dependencies, I guess @aspiers just wanted to pick out the valuable bits in docs/mkcloud.md
I am over it now

@aspiers
Copy link
Contributor

aspiers commented May 4, 2015

@jdsn I was only talking about the bits which are obviously right.

@aspiers
Copy link
Contributor

aspiers commented May 4, 2015

@MaximilianMeister are you "over it" or "all over it" ?!

get rid of duplicated line
@MaximilianMeister
Copy link
Contributor Author

@aspiers :-) lets say I don't cry anymore... so I am over it

well... now I am done ;-)

reviews welcome

@aspiers
Copy link
Contributor

aspiers commented May 4, 2015

Haha :-) Great, thanks! 👍

@rsalevsky
Copy link
Contributor

The changes looks good to me. 👍 If this is the final solution we will see in the future.

MaximilianMeister added a commit that referenced this pull request May 5, 2015
@MaximilianMeister MaximilianMeister merged commit 3cfc26d into SUSE-Cloud:master May 5, 2015
@MaximilianMeister MaximilianMeister deleted the doc-enhancements branch May 27, 2015 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants