search

jreybert / vimagit

Ease your git workflow within Vim
1.31k stars 49 forks source link

refactor: multiple `s:print` calls into one to print given list #127

Closed argshook closed 7 years ago

argshook commented 7 years ago

@jreybert so im learning vimscript. this is what i have in mind with "single call with list" in this comment https://github.com/jreybert/vimagit/issues/123#issuecomment-283509881

this doesn't include all places where calls to s:print could be reduced, more things could be refactored i think

jreybert commented 7 years ago

Good PR! I am still not sure that I'll merge this branch.

Indeed, more thing could be refactored. But in stage sections, you will face that vimagit keep the line of each file/hunk header, it won't work as is just pushing in arrays .

argshook commented 7 years ago

@jreybert mind sharing the reason why not to merge? performance or something else?

just fyi, i've been on this branch for almost a week now and didn't notice any issues. Content starts nicely from the top :)

jreybert commented 7 years ago

@argshook no real reason. I just did not work on vimagit last days. I merge this in dev/issue_123, and I'll merge it in next.

For your next PR, please prefer rebase instead of adding "Fix previous commit" commits.

Thanks again for your commit.

argshook commented 7 years ago

Awesome, this is great, thank you!

regarding rebasing, i always do that so i'm guessing you meant amend. i probably should've done it but didn't cause i had already pushed some commits prior to fixes and didn't want the notorious -f, i avoid it everywhere as much as i can.

To avoid littering git history with fix commits like mine, it's possible to squash and then merge, so PR merges look like one commit.

in any case, i'm honored to be part of vimagit :)

jreybert commented 7 years ago

git push -f, when used wisely, is very very good! For vimagit, I ask PRs to be rebased instead of adding "fix previous commit" commits. (of course, if your PR needs splitted commit because of different stuff, you can/must add several commits).

As long as every developer on a dev branch are OK with rebasing, it is a good thing to use rebase in a project (it may depend on project leader of course).