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 needs clearly defined interface #220

Open
aspiers opened this issue Feb 18, 2015 · 7 comments
Open

mkcloud needs clearly defined interface #220

aspiers opened this issue Feb 18, 2015 · 7 comments

Comments

@aspiers
Copy link
Contributor

aspiers commented Feb 18, 2015

Currently mkcloud relies on a bunch of shell environment variables to control its behaviour:

  • Some of these are documented via comments near the top of the script.
  • A smaller number are documented in docs/mkcloud.md.
  • Some are documented in the usage text output when you run ./mkcloud with no arguments. (It's also supposed to be output when you run ./mkcloud help, but that's broken.)
  • Many are not documented at all.
  • Some of the variables have documentation duplicated in more than one of the three locations, and sometimes the duplicated info is inconsistent.
  • These environment variables are mostly (but not all!) lowercase, and are indistinguishable from other "private" shell variables used within the script. One of the causes of this is violation of the common shell coding standard which uses uppercase for environment variables and constants, and lowercase for local variables.
  • Some variables are optional and others are mandatory, but it's not always clear which are which.

How did we get ourselves in this mess? I suggest that the answers include (but are not limited to) the following reasons:

  • shell code does not encourage clean encapsulation
  • the "boiling frog" effect: scripts start as quick small hacks, and gradually grow past the point where they should be treated like proper production code without anyone noticing
  • no structured peer review across the whole team for a large part of the code's history

This is not the fault of any one person, and anyway my goal is not to blame people. The priorities are:

  1. to learn how we can avoid this in the future, and
  2. to figure out how we can clean up the mess.

My suggestions are:

Thoughts welcome.

@MaximilianMeister
Copy link
Contributor

I personally had to spend some (and some more) time with @jdsn and @bmwiedemann to find my way around in this 2 main scripts.
I agree with the following:

  • documentation can always be improved, but that goes for crowbar and other software too, (lately we had already one round of documentation enhancement)
  • the variable casing is somewhat unclear, but it can either be documented and/or improved too.
  • review should be done over github workflows (commenting etc) even if it takes more time.

So yet i dont see a big problem nor a mess here, besides the mess in my mkcloud working directory ;)

@aspiers
Copy link
Contributor Author

aspiers commented Feb 18, 2015

@MaximilianMeister One big problem is that I've wasted a lot of time whilst learning mkcloud, because the docs were so bad (and BTW for a long time they did not even exist). And if it happens for me then it can happen for other mkcloud users in the future. It's inefficient to have to rely on @jdsn and @bmwiedemann all the time - if that approach was OK, we wouldn't need to ever bother documenting anything, we'd just ask the authors.

@MaximilianMeister
Copy link
Contributor

i didn't say that this is an approach to go to @jdsn and @bmwiedemann. this should not be needed. i just gave you my background. (and i am sitting next to them, so it was easy for me to reach them)
your many recent enhancements and the ones from others about readability and documentation make it easier for the next ones working with it. we should definitely push it further

@aspiers
Copy link
Contributor Author

aspiers commented Feb 18, 2015

OK we are agreed :)

@aspiers
Copy link
Contributor Author

aspiers commented Mar 22, 2015

This is being addressed gradually thanks to productive discussions during the Cloud team's workshop in March 2015:

@MaximilianMeister
Copy link
Contributor

two of the gradual refactoring steps are #335 and #343

@aspiers
Copy link
Contributor Author

aspiers commented Sep 5, 2016

@vuntz, @sidthe Fixing this mess becomes more urgent now that

a) the team is growing faster than ever
b) mkcloud is starting to be used externally

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

No branches or pull requests

2 participants