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

Adding option to add untracked files before a commit is performed. #87

Closed
wants to merge 2 commits into from

Conversation

meenie
Copy link

@meenie meenie commented Jul 24, 2014

As the title says :). It's something we need for our internal projects. If you don't want to merge, we'll just use our forked version.

@meenie
Copy link
Author

meenie commented Jul 24, 2014

@joshua703 - I have added in the extra functionality to report out which files that have been added.

@thomaswelton
Copy link

👍 thanks I've installed your fork until this PR lands

@eddiemonge
Copy link
Collaborator

This is a pretty scary feature. Its telling the system to add something that you may not know is there. Adding changed, tracked files is at least safe because they are already tracked so worst case is they are completely different. Untracked could accidentally add lots of unwanted data. What kinds of scenarios are you doing with this?

@meenie
Copy link
Author

meenie commented Aug 20, 2014

In my grunt tasks, all of my compiled files, as well as assets (images, fonts, etc) are copied to a ./dist folder. The issue I had was when new assets being added and then copied to the ./dist folder for deployment weren't being added. This change makes it so you can opt-in (using the addUntrackedFiles option) to auto-adding all files, even if they are new. Then at the end, it will tell you exactly which new files were added.

I'm not worried if this PR doesn't land. If it doesn't, we'll just keep using my fork, so no worries :). Just thought I would give it to the community to see if they wanted it.

@eddiemonge
Copy link
Collaborator

Forget my previous question. This is still a scary feature but it is one that is potentially needed for things like release files only existing in tags and not main repo. Let me think about this and see if there isn't a better way this could be done. Right now Im thinking you have to manually specify files using grunt file matchers would be a good compromise

@meenie
Copy link
Author

meenie commented Aug 20, 2014

It's only dangerous based on how you have your grunt config setup. I tell grunt bump that I only want to add/commit files in my ./dist folder as well as the bower.json and package.json. Anything that goes in the ./dist folder is something that I absolutely always 100% want to be deployed. I specify in my grunt config all of the files that I want to go in there. If something is missing, it will break my map. So you could say that not having this option is more dangerous than not having it.

@ZuBB
Copy link

ZuBB commented Sep 11, 2014

Let this plugin does its job. If you have additional tasks in VCS field, do them with proper tools. you can use one of grunt plugins that work with git: grunt-git or grunt-git-them-all(yes, a tiny bit of pr)

@meenie
Copy link
Author

meenie commented Sep 11, 2014

@ZuBB: If that's the case, then grunt bump shouldn't do any git work at all. It's already built to do a git add. This PR fixes it to add untracked files as well. Like I said previously, if this is not something that gets merged in, that's cool; we can still use our fork. But I figured others might want this as well.

@ZuBB
Copy link

ZuBB commented Sep 11, 2014

@meenie actually it build in way that allows to do any git command. 'add' command is omitted with help of "direct" commit.

But that's not a deal. you come with pr because you have untracked. @LeMisterV wants a '-f'. Someone else will come with need to handle files that were deleted ahead of 'git rm'.

this queue may be endless. we ought to use commit, tag, and push. but IHMO would be better to stop on those three

@meenie
Copy link
Author

meenie commented Oct 28, 2014

@eddiemonge: Have you thought about this?

@eddiemonge
Copy link
Collaborator

I think if there was a dry-run option then this would be better. While I do see how a need for this can exist, it still scares me. I'm actually dealing with something similar in a project at the moment and I grunt the build process, add the new files, then do a full release cycle. How I have it is not ideal since I am checking in the built files into git main instead of a dedicated branch that a tag could point to but doesn't need to exist.

@meenie
Copy link
Author

meenie commented Oct 28, 2014

I think it's more scary that someone doesn't know exactly how their build process works and somehow grunt bump commits a massive file that was previously .gitignored. In my grunt config, the directory that I am telling grumt bump to commit is the ./dist directory. This is built by grunt, strictly created by the config I've come up with. I could also take it one step further in that the release process could clear out any files in the ./src directory that aren't already tracked so nothing can "leak" into the build process that hasn't already gone through code-review.

@rpocklin
Copy link

I came across this PR when wanting to stage and commit changelog-<version>.txt running git changelog in the middle of the grunt-bump process (as per the README.md). IMO this PR is a bit dangerous - can't we just git add the contents of options.commitFiles which is an explicit list, before the task calls git commit?

Sure I can (and have) added grunt-git but it's yet another library :(

@eddiemonge
Copy link
Collaborator

@rpocklin you have a separate changelog for every version?

I still don't know about this. I try to close it and type up the reasons why and then immediately counter them but am still afraid of the implications of this. There has to be a better way. I think if the added files were part of a grunt/glob match then I would feel far more comfortable. Then you can specify which files would be generated that are to be added instead of adding everything blindly. Yeah, I am going with that. If you want to change this so it uses a pattern matcher then I'll accept it. Until then, I am closing this one.

@eddiemonge eddiemonge closed this Jan 28, 2015
@meenie
Copy link
Author

meenie commented Jan 29, 2015

@eddiemonge Fair enough :). That sounds like a great addition to this PR.

@eddiemonge
Copy link
Collaborator

If you have time and the motivation to do it, I encourage you to update the pr with that functionality. If not, want to make an issue for someone else to tackle it?

@meenie
Copy link
Author

meenie commented Jan 29, 2015

I can't 100% commit to it, so might as well create the issue to track it. If/when I update this PR I will link to it.

@joshuahiggins
Copy link

Ah, I didn't notice this thread before submitting my own pull request, #120, which seems to mimic @meenie's nearly identically.

On that note, I agree with @meenie's statements in this thread. This repo has already made the assumptions that it can add, commit, and push your git process, so it needs to offer the ability to control the finer aspects of adding, committing, and pushing. If someone's using an untracked option without understanding what it's doing, you can't feel responsible for that. I personally think push and pushTo are far scarier options then just committing untracked files.

@mischah
Copy link

mischah commented Sep 1, 2015

Hi,

I would love to see this merged. But agree that it could be safer to make use of glob match to gain a little bit of control what is added.

Background:
I have the same usecase as @meenie.
My release tasks are generating files in my dist directory which is under version control.
When there are new files (which are already added and commited in my src directory) they are untracked in dist after running one of my release tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants