radiovisual / timecard

Keep track of your project development time.
MIT License
20 stars 4 forks source link

add .timecard.json to .gitignore #5

Closed mpfeil closed 8 years ago

mpfeil commented 8 years ago

Adds a new argument append to the CLI for appending .timecard.json to an existing .gitignore file in the current working directory.

The append function is also called during the new process after the .timecard.json was successfully created.

radiovisual commented 8 years ago

thanks for this -- it's a good idea. I will be away from my computer all day tomorrow, but when I return I will test it out and take a closer look.

radiovisual commented 8 years ago

Overall, I like the feature, so if you don't mind removing that one console.log() and changing the phrasing a little bit in your messages, I will merge this. Thanks!

mpfeil commented 8 years ago

Great you like it. I don´t mind changing the what you mentioned. Will get back to it tomorrow.

radiovisual commented 8 years ago

awesome. thanks! :+1:

mpfeil commented 8 years ago

I´ve changed the message and removed the console.log for logging the current working directory.

radiovisual commented 8 years ago

Thanks. I just took a look at the new changes, and I caught a bug. Because the .timecard.json file is already in the .gitignore file by default, if you answer "Yes" to .gitignore found: do you want your timecard to be kept in version control? then you get this message:

GITIGNORE: The .gitignore file already contains .timecard.json. Nothing was appended!

...and it leaves the entry of timecard.json inside the .gitignore file.

Instead, if you answer "Yes" to "do you want timecard under version control?" it should remove .timecard.json from the .gitgignore and report something like .timecard.json was removed from your .gitignore file

mpfeil commented 8 years ago

Thanks. I just took a look at the new changes, and I caught a bug. Because the .timecard.json file is already in the .gitignore file by default, if you answer "Yes" to .gitignore found: do you want your timecard to be kept in version control? then you get this message:

GITIGNORE: The .gitignore file already contains .timecard.json. Nothing was appended!

I would say that this behavior is correct. If you answer "Yes" you want to add timecard to .gitignore file.

Instead, if you answer "Yes" to "do you want timecard under version control?" it should remove .timecard.json from the .gitgignore and report something like .timecard.json was removed from your .gitignore file

Here I would suggest: if you answer with "No" and .timecard.json exists in the .gitignore file then the entry should be removed.

radiovisual commented 8 years ago

I would say that this behavior is correct. If you answer "Yes" you want to add timecard to .gitignore file.

But the question is "do you want the timecard to be under version control?", not "do you want to add 'timecard.json' to the .gitignore file?" these are two different things.

So when someone says "YES, I want my timecard under version control", then it means that .timecard.json should not be in the .gitignore file (because they want the .timecard.json file to be tracked by git), and therefore removed from the .gitignore file (if it exists).

mpfeil commented 8 years ago

Sorry mixed up timecard and .timecard.json. Will fix it tomorrow.

radiovisual commented 8 years ago

Just a heads up: I have added some new changes to this project, so if you decide to do more work on this PR, then please remember to pull in the latest changes. The most significant change has been the addition of XO linting which effects every source code file in the project, and I have set things up to start writing unit tests and added support for travis. Thanks!

radiovisual commented 8 years ago

As you may have seen, a lot of changes have taken place. I ported the codebase over to ES6, added a new cli, added unit tests and closed a bunch of tickets. So you will need to pull in the new changes if you still want to work on this feature. Otherwise, if you don't have the time (or desire) we can close this PR, and open an issue and I can work this feature into the codebase when I get some free time.

mpfeil commented 8 years ago

I saw your changes. Right now I don´t have time to work on my PR. Next week there is enough free time to work on it. Hope that´s fine for you.

radiovisual commented 8 years ago

That works for me. No rush. Thanks!

radiovisual commented 8 years ago

@mpfeil , if you have time to work on this, I will leave this PR open, otherwise, if you don't have time (or interest) I can close this PR and open an issue to add the feature later on.

radiovisual commented 8 years ago

The fork for this PR has been deleted, so I am closing this PR due to inactivity. If you want to add the feature later on, you are more than welcome. Thanks.