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

Prompt before the initial clone of fips in the fips template. #242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomcc
Copy link

@thomcc thomcc commented Oct 16, 2019

My first encounter with fips was having it clone a repo and drop a bunch of folders in a directory I cared about when I was trying to follow the build steps of an unfamiliar project.

This was a bit surprising, and I would have appreciated a confirmation prompt at the start of all of it, even if it wasn't that bad to rectify.

Maybe this will help some future users will avoid such issues. Or even myself from accidentally doing the same thing again.

Sorry in advance, I'm not good at python. I made an attempt to stick to your style (well, I put spaces before the : anyway), but might have messed up.

@floooh
Copy link
Owner

floooh commented Oct 16, 2019

Hi, I agree that this is a problem, and generally like the idea of a prompt, and you also thought about hinting automated build systems with the environment variable, but:

  • all changes to the fips template file are problematic, because this is 'decentralized' and copied into each project, for this reason it should remain as small and simple as possible
  • ...which would mean: the prompt should move into the fips project itself before dependencies are pulled (so the fips project would be pulled first and this would "pollute" the directory, but I think that's ok, it's the (potentially) dozens of dependencies which are the problem)
  • I wonder if there's a better way to detect whether a confirmation should happen or not than via an environment variable...

But anyway... I'm hesitant merging this PR also because of another thing that's been rolling around in my head for a long time, but never got around to actually do it:

I'm thinking about changing fips' default behaviour so that everything is written into a subdirectory of the project (for instance ".fips-build"), so that it doesn't pollute the directory level above that project. The current behaviour would become a special "shared depdencies mode" which would need to be enabled per project (for instance through a new ".fips-settings" flag). That way a warning/confirmation wouldn't be needed, because by default only this local subdirectory would be written to.

@thomcc
Copy link
Author

thomcc commented Oct 16, 2019

I wonder if there's a better way to detect whether a confirmation should happen or not than via an environment variable...

I considered:

  • listing the parent directory and seeing what it had in it (although I wasn't exactly sure what the check would be -- certainly if this is the only item it's probably fine, or if the parent dir is something like fips-workspace, but these kind of heuristics are finnickey)
  • checking istty on stdout (although IDK if this works on windows)

But even then, I think for use cases like CI or other automation, you'd still want to be able to disable that kind of thing absolutely.

all changes to the fips template file are problematic, because this is 'decentralized' and copied into each project, for this reason it should remain as small and simple as possible

I could imagine eventually fips evolving support for updating the fips template, e.g. as part of a fips update fips or something. You could even be intelligent and only overwrite it if it hasn't changed (easy enough to check with git, even though they're in different repos, or fips could keep a list of all past template/fip's file shas and compute the current).

That said this isn't exactly critical functionality that people can't live without -- it's nice if new projects can get it, but it's not like not updating it breaks backwards compat or anything.

I'm thinking about changing fips' default behaviour so that everything is written into a subdirectory of the project (for instance ".fips-build"), so that it doesn't pollute the directory level above that project

This would be wonderful. The pollution of the parent dir is really annoying and surprising for new users (it and the use of submodules are my two least favorite things about fips).

@floooh
Copy link
Owner

floooh commented Oct 17, 2019

I could imagine eventually fips evolving support for updating the fips template, e.g. as part of a fips update fips or something.

That's a good idea!

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.

2 participants