-
Notifications
You must be signed in to change notification settings - Fork 38
Consolidate to a single .gitignore in the root directory #106
Comments
I think you may not want to do this. If you do, the One option some folks do to work around this is to create an empty While I do favor a single root .gitignore file in general, the empty directory scenario is a reasonable exception to the rule in my view. .gitkeep isn't a terrible choice either though (it's a fairly common pattern). You could even put a comment in the file describing what the empty directory is for (using comment syntax, including plaintext, since nothing will be reading that file). |
Thanks for the comment. It inspired me to think about our real goal with all of this. Currently we track However, it would force users not only to create Using a |
I think the issue with a normal (non dot) file is that if you wanted to read multiple CSVs at once (which is why the directory exists in the first place), that file would get sent in over stdin if you relied on shell globbing (which I think is a natural way to do it). Just to be clear what I mean by that, $ echo 1 > .1
$ echo 2 > 2
$ cat *
2 vs $ echo 1 > 1
$ echo 2 > 2
$ cat *
1
2 So if I were using
Taking a further step back: why have this directory in Git at all? All Starfish knows about is what comes in on stdin. I don't normally add directories to Git unless there are expectations in the code about the directory structure. I've accidentally committed a change to an existing file I didn't intend to, but I've never in my life accidentally done |
Good question. I agree that ideally our tracked file structure would match only what the code requires, but in this case I'm prioritizing ease of using Starfish over code clarity for devs. Since Starfish is a community management tool, I'd love for non-engineers to be able to follow the README instructions and run it, so I'm making everything as explicit as possible. So we have the empty I'm assuming that some users will want to have multiple CSVs in a separate directory. Having the empty Since this folder will likely contain PII I also want tracking of the files turned off by default. |
If you wanted to do it that, what do you think of just having the directory hard-coded and read all CSVs in that directory? |
We definitely want to read from a file path rather than STDIN. It's on the to-do list for when we circle back to starfish. One interesting option (which I'm not advocating for at this point) would be to prevent users from reading the file from anywhere inside the starfish git working directory tree. Basically, prevent them from accidentally committing it. My first thought was to prevent it from being in any git tree, but they might specifically want to keep the list tracked in an internal git repo, so that seems too restrictive. I suppose we could add a command line option to override that (--allow-input-file-to-be-in-a-git-tree), but I don't love that. |
Oh yeah, right! I've even created an issue for that. |
@danisyellis Yeah, I noticed that after I'd already left the last comment. @kevindigo I was curious how you'd prevent being in any Git tree. Keep moving up the directory tree until root (or you find a .git)? I know you weren't necessarily advocating that, just thinking through what that'd mean. |
Yes, I was imagining climbing the tree looking for .git files. Good point about the repo with no remotes. |
We currently have a second .gitignore file in CSVsToParse. While that works, it is a bit unusual to have subtree .gitignore files. Plus, we have to then have a line in that .gitignore telling it not to ignore .gitignore itself. It seems cleaner to just get rid of the extra .gitignore file, and have all of the ignore rules in the main .gitignore file in the root directory.
The text was updated successfully, but these errors were encountered: