Automatically create ".stignore" file if does not exist #1169
Conversation
|
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Codecov Report
@@ Coverage Diff @@
## master #1169 +/- ##
==========================================
- Coverage 34.57% 34.26% -0.31%
==========================================
Files 65 66 +1
Lines 5440 5489 +49
==========================================
Hits 1881 1881
- Misses 3355 3404 +49
Partials 204 204
Continue to review full report at Codecov.
|
| continue | ||
| } | ||
|
|
||
| if err := askIfUpdatingStignore(folder.LocalPath, stignorePath, gitPath); err != nil { |
There was a problem hiding this comment.
this is going to cause that we ask the user every time. I don't think we should do this unless we can do it only once per repo. I think that adding the .git to the .stignore we generate is good enough without being annoying about it.
There was a problem hiding this comment.
I will handle to ask only once. This is important, I see lots of users without .git on their .stignore file, which makes okteto up very inestable.
There was a problem hiding this comment.
do you know how this users are creating the .stignore? are they doing it by hand, or getting it from our samples / init command?
There was a problem hiding this comment.
must be manually because we always include .git
cmd/up/stignore.go
Outdated
|
|
||
| if !answer { | ||
| stignoreContent := "" | ||
| if model.FileExists(gitPath) { |
There was a problem hiding this comment.
let's just put it all the time, it's pretty standard to have it as part of an ignore list.
| if model.FileExists(gitPath) { | ||
| stignoreContent = ".git\n" | ||
| } | ||
| if err := ioutil.WriteFile(stignorePath, []byte(stignoreContent), 0644); err != nil { |
There was a problem hiding this comment.
if the user chose no as the answer, why do we still create the ignore file?
There was a problem hiding this comment.
It servers two goals:
- only ask once
- more discoverability of the feature
There was a problem hiding this comment.
but it goes against what you're asking the user. if they are asked "should I create the stignore file", they say no, and the file is still created they are going to think there's a bug. if you want to always create the file, then don't ask them (I don't think this is right approach btw).
There was a problem hiding this comment.
what do you suggest?
There was a problem hiding this comment.
if the user says no, we don't create the file, and don't ask them again for that repo.
There was a problem hiding this comment.
I understand the painpoint, but we need to find a way to educate the user rather than ignore their answers. I think we should explain why synchronizing .git is a bad idea, but if they want to do it, it's their choice.
There was a problem hiding this comment.
Right, but the implementation of "don't ask them again for that repo" must live in the repo. If not, every time a user clones the repo will get prompted, and that feels wrong because okteto has been already configured.
I will rephrase the question to make it clear.
There was a problem hiding this comment.
It did look like a rephrasing issue. I like it better now, but maybe we should also state the benefits of doing so. What about something like:
Okteto requires a '.stignore' file to intentionally ignore file patterns that help optimize the synchronization service.
| if err != nil { | ||
| return fmt.Errorf("failed to get language for '%s': %s", folder, err.Error()) | ||
| } | ||
| c := linguist.GetSTIgnore(language) |
There was a problem hiding this comment.
let's make sure all of them include .git
pchico83
force-pushed
the
stignore
branch
2 times, most recently
from
951d63b
to
9120c7d
Compare
Signed-off-by: Pablo Chico de Guzman <pchico83@gmail.com>
Co-authored-by: Ramón Lamana <rlamana@gmail.com>
Proposed changes
.gitis defined in the.stignorefile