eficode-academy / git-katas

A set of exercises for deliberate Git Practice
MIT License
1.32k stars 860 forks source link

Set local git user name and email after initializing exercise repo #233

Closed schiluveri closed 4 years ago

schiluveri commented 4 years ago

Set local git user name and email after initializing exercise repo (to distinguish commits).

schiluveri commented 4 years ago

I think we should use a value for user.email that looks more like a valid email address for pedagogical reasons. maybe something like bot@git-katas.fake ?

I can use this .

Also, I am pretty sure that we need a clean-up step as well that does a git config --local --unset user.... to remove the value again so future commits are made with the users own identity.

As far as I can work out, this will require that every single kata is then modified with a call to the clean-up function - but once that is done, we can easily add more setup/clean code if we find later needs.

I feel, unless missing any other point, it's not necessary. These two statements are executed as part of the 'exercise' repo that's created using makerepo() in respective katas' setup scripts.

Configured values are stored under 'exercise/.git/config', so this is fail-proof in 2 scenarios - 1) if one wants to redo the same exercise, executing setup script ensures it first deletes the 'exercise' folder before initing a new one, and 2) since those were local configs, it won't affect values at any other level.

schiluveri commented 4 years ago

Clearly, it needs an efficient plan on how cleanup or tearing down part should be handled. Or implement this in setup scripts of every kata.

JKrag commented 4 years ago

I am thoroughly confused by the GH user interface this morning, and can't find my original review comments on this issue.

I am happy with the email change, but as far as I can tell, we haven't yet addressed the cleanup issue. If we don't unset the "bot user", then all the following commits made by the "student" will also be made with that identity, and that sort of defeats the whole purpose of separating the two.

@schiluveri If this is too big a change, please let me know, and I will make a stab at it myself (at least in the Bash version).

JKrag commented 4 years ago

By the titel of this PR, cleanup is not included, but then we need a second PR to fulfill #195 and in my opinion, one doesn't make sense without the other.

RandomSort commented 4 years ago

I think this is broken without the clean up. It is also a potentially breaking change :) I will go through the exercises and perhaps add the cleanup to this. I should be able to find some time for this this week.

schiluveri commented 4 years ago

I am thoroughly confused by the GH user interface this morning, and can't find my original review comments on this issue.

Perhaps, you might want to go back to your comments on the issue itself. We're discussing this on PR review page :)

I am happy with the email change, but as far as I can tell, we haven't yet addressed the cleanup issue. If we don't unset the "bot user", then all the following commits made by the "student" will also be made with that identity, and that sort of defeats the whole purpose of separating the two.

@schiluveri If this is too big a change, please let me know, and I will make a stab at it myself (at least in the Bash version).

Yes, clean-up part can be added; I'm on that now. Agreed with @sofusalbertsen now that, initially, configuring local trainer username and then clean-up can be added to few katas from here https://github.com/praqma-training/git-katas/blob/master/Overview.md

For extensibility and easier maintenance, I use your suggested methods for this:

setup() -- a separate function to in utils to configure local user name and email

and teardown() -- to unset the values.

JKrag commented 4 years ago

I have approved, but I would still prefer a 👍 from either @RandomSort or @sofusalbertsen before merging.

And when this is merged, we should also create a new issue for the Powershell version of these changes.