Update docs for table builders #121
Conversation
| We recommend creating a git repo per study, to help version your study data, which | ||
| you can do in the same directory as the manifest file. | ||
| you can do in the same directory as the manifest file. If you've forked your study from | ||
| the template, you've already checked this step off. |
There was a problem hiding this comment.
If the create command just started a git dir for ya, you could drop this paragraph. Should the create command just clone the template repo?
There was a problem hiding this comment.
I don't like the idea of trying to manage a user's git infrastructure, honestly (esp with the above comment about maintaining multiple paths) - I'll take a paragraph of text.
There was a problem hiding this comment.
git infrastructure is not what I meant to suggest - or maybe we're just using different words for things.
If create makes a new directory with your new study files, it doesn't seem unreasonable to also do git init and git add *; git commit -a -m "initial commit" for them. I wouldn't have thought of that as managing their git infrastructure.
There was a problem hiding this comment.
a user could, theoretically, install the library from pip and not worry about git until later.
i could try to detect if git is present or not? i dont know, i'm still not crazy about it.
There was a problem hiding this comment.
Sure, was just a suggestion - ignore it if it doesn't fit
There was a problem hiding this comment.
I'm not going to do this as part of this PR, but I created a task around this: #122 - if you want to add any notes, please do.
docs/creating-studies.md
Outdated
| - You may want to select a SQL style guide as a reference. Mozilla provides a | ||
| [SQL style guide](https://docs.telemetry.mozilla.org/concepts/sql_style.html), | ||
| which our sqlfluff config enforces. |
There was a problem hiding this comment.
This is a bit confusing to me - am I free to pick a new style guide, or will cumulus-library enforce Mozilla's on me? Could we just say "Recommended: go read the mozilla study guide, we will enforce it"?
There was a problem hiding this comment.
You can elect to not run SQLFluff - this is the current state here for non-covid studies. But if you do use it, that's what SQLFluff enforces
There was a problem hiding this comment.
Right, I was trying to suggest that this language could be cleaned up to make that state of things clearer. Like, "By default, we use sqlfluff to enforce Mozilla's guide here. If you're a crazy person and want to go change that, you can and here's how" - but I don't know how much you want to point people towards shooting their own foot anyway
There was a problem hiding this comment.
Clarified this a little bit, which also makes the next line about github's guide make more sense.
This PR makes the following changes:
Checklist
docs/) needs to be updated