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

Curifactory assumes that it is being run inside a git repository #79

Closed
deniederhut opened this issue Sep 12, 2023 · 3 comments
Closed
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects

Comments

@deniederhut
Copy link

Is your feature request related to a problem? Please describe.
curifactory appears to call the git CLI, which results in a lot of errors if not inside a git repository

Describe the solution you'd like
It would be nice if this was added to the getting started guide

Describe alternatives you've considered
Another option would be checking for .git, then skipping whatever git-based checks curifactory is doing if not present

Additional context
Running an experiment outside git results in

fatal: not a git repository (or any of the parent directories): .git
warning: Not a git repository. Use --no-index to compare two paths outside a working tree
usage: git diff --no-index [<options>] <path> <path>
@WarmCyan
Copy link
Collaborator

Hmm, yeah git is a useful assumption (primarily for reproducibility, since we can store the current git commit hash alongside any experiment run info), but you're right, we should definitely call that out in the documentation.

I think we could also handle non-git-repo situations a little more gracefully and just show a single curifactory warning in the log to bug the user to use git (rather than dumping out a bunch of git errors), does that seem reasonable?

@WarmCyan WarmCyan added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 18, 2023
@deniederhut
Copy link
Author

I think we could also handle non-git-repo situations a little more gracefully

I mean, I think that's up to you? If having source control is really helpful, maybe instead you want to require it and then quit if you are not in a repo?

But yes please do add it to the docs if that is the intended usage.

@WarmCyan
Copy link
Collaborator

I mean, I think that's up to you? If having source control is really helpful, maybe instead you want to require it and then quit if you are not in a repo?

Yeah, I personally tend to be against hardline preventing users from shooting themselves in the foot, so long as said foot-shooting is understood and intentional 😁

Since Curifactory doesn't have hard restrictions about running in other non-reproducible ways (interactive environments etc), for consistency I'll just make it spit out a warning

But yes please do add it to the docs if that is the intended usage.

Will do!

@WarmCyan WarmCyan added this to Next in Roadmap Sep 19, 2023
@WarmCyan WarmCyan moved this from Next to WIP in Roadmap Sep 19, 2023
@WarmCyan WarmCyan moved this from WIP to Done in Roadmap Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants