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

VCS commit should allow selecting files #128

Open
OdyX opened this issue Oct 18, 2017 · 5 comments
Open

VCS commit should allow selecting files #128

OdyX opened this issue Oct 18, 2017 · 5 comments

Comments

@OdyX
Copy link
Member

OdyX commented Oct 18, 2017

I have a workflow in which I have plenty of non-committed files in my working directory: project files for my IDE, ngrok copy, WIP for stuff, etc.

Running git add --all in https://github.com/liip/RMT/blob/master/src/Liip/RMT/VCS/Git.php#L70 is bad and should be discouraged.

Please at least permit a list of files to be provided.

@jeanmonod
Copy link
Member

jeanmonod commented Oct 18, 2017

This it why we strongly recommend to use the working-copy-check as a prerequisite action. I have at least 2 projects were the list of generated files at release time is not determinable upfront. But I strongly agree with you that there is some danger out there.
What we could do is to add, as an option, a fixed list of file on the commit action. Wdyt?

@ppetermann
Copy link
Contributor

Ideally you have a clean workspace when you run RMT.

the ideal situation is:

  • your ide's project files are in your global gitignore, where they belong.
  • your changes are committed, with proper commit messages and everything
  • WIP at least stashed (why do you work on several things on the branch you release from in the first place, you should only be merging stuff over thats supposed to be released)

@OdyX
Copy link
Member Author

OdyX commented Oct 18, 2017

What we could do is to add, as an option, a fixed list of file on the commit action. Wdyt?

That's good. Ideally something that can be piped to git add directly.

@OdyX
Copy link
Member Author

OdyX commented Oct 18, 2017

@ppetermann Sure, in an ideal world.

I stand to saying that adding all of the workspace, especially on release time, is, at best, error-prone.

Being specific, especially with git, is always better than being generic. I, for one, always use git add -p and only commit what I want. My tree is constantly cluttered with non-committed patches (e.g. a debug stanza I want to test across multiple branches). And that works fine for all git-related purposes. Releasing a project should not assume that all I have in my tree is targeted at said release.

@ppetermann
Copy link
Contributor

ppetermann commented Oct 18, 2017

I stand to saying that adding all of the workspace, especially on release time, is, at best, error-prone.

Well, I believe that RMT shouldn't be committing anything that it hasn't touched itself, but that doesn't mean that the current state is more error-prone then what you are doing - infact, as @jeanmonod already pointed out, there is a check for clean working space, which should prevent people from having stuff committed thats in the wrong place.

however you seem to be under the impression that something that you describe yourself as "cluttered" is positive, and it really isn't. neither is entering lists manually each time you release - your tests will run against the stuff with the files there, the release won't have the files - you forget anything, there is no warning signal anymore.
and then there is the thing that git has stash build-in for the exact thing of temporary stashing away changes, which even if you don't follow a workflow where you have a dedicated branch for releases, you can commit what you want in the release, then stash everything else, run RMT, which runs the test clean on the same state thats the one that will be released, makes the release, and then you can pop it off the stash again - not the most professional workflow, but much better than turning off checks, and having tests that might include stuff that you could forget in your release list.

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

No branches or pull requests

3 participants