-
Notifications
You must be signed in to change notification settings - Fork 580
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
New actions for GitCommitBear #2927
base: master
Are you sure you want to change the base?
Conversation
bears/actions/AddNewlineAction.py
Outdated
Add (N)ewline | ||
""" | ||
new_commit_message = '{}\n\n{}'.format(self.shortlog, self.body) | ||
command = 'git commit --amend -m "{}"'.format(new_commit_message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abhishalya I think git commit --amend
also adds the staged changes. This is not good. Any alternatives?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess git rebase
would do then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at this. Even rebase is not ideal if someone is using your commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but here we are expecting the user to change the commit message. If the commit is not pushed doing rebase is just fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can warn that history will be rewritten action is applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, a warning message would be good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess
git rebase
would do then.
git rebase
doesn't run when there are unstaged or uncommitted changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git commit --amend -o
will do the job.
775f5d0
to
31fff83
Compare
8ecf188
to
b17afae
Compare
51e9f5e
to
bd53589
Compare
|
original_file_dict, | ||
file_diff_dict, | ||
applied_actions=()): | ||
new_message, _ = run_shell_command('git log -1 --pretty=%B') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User can first apply EditCommitMessageAction
and change the commit message, so to check if AddNewlineAction
is applicable or not, commit message is extracted first.
bd53589
to
00a3374
Compare
00a3374
to
26fad10
Compare
Travis is failing because it depends on coala/coala#6029 |
26fad10
to
629bc53
Compare
Yeah, please add the coala requirement as your branch. |
Btw this is only temporary, to check if the changes work fine. |
I made these changes, am I doing something wrong because the first travis job is failing because of some weird reason but it was passing before. |
You can ignore the first job of Sentinel since it fails for moban. But, the rest should pass... Looks like a breaking test. |
Btw, I think the rest of the jobs would stop if Sentinel fails (which it would eventually since the first one would fail due to moban) so instead of restarting those you can also make relevant changes to travis. |
Look at the error of the second build. This is because the changes in coala/coala#6029 are not there yet. |
coala is being installed correctly there from your branch, let me try this out locally |
@akshatkarani This seems to be the problem... |
f567bff
to
b15ec55
Compare
Removed the first sentinel build, now only one build is failing and the error is the same. |
b15ec55
to
4c5c866
Compare
Okay build is passing with these temporary changes. |
Nice, (since other mentors aren't active) give me some time to check this (and the core changes) in more detail. |
4c5c866
to
27b2e32
Compare
Ping @Makman2 |
|
This PR depends on coala/coala#6029, so for builds to pass this commit has been added. Once that PR is merged it can be removed.
Yes, I will. |
8b28fcc
to
675cc1e
Compare
@Makman2 I have separated the commits properly. |
Temp commit not required now, also needs rebase. |
This PR still depends on coala/coala#6029 😕 |
675cc1e
to
5b1a03c
Compare
24f27bc
to
d9a5df2
Compare
#2855 seems related to moban sync. Also, the commit should be a part of separate PR. |
d9a5df2
to
bc41a68
Compare
bc41a68
to
892e753
Compare
This adds a new action for GitCommitBear. When applied it opens up a editor for user to edit commit message of the HEAD commit. This also makes changes is CommitBear.py to pass EditCommitMessageAction when Result is yielded.
This adds a new action which adds a newline between shortlog and body of commit message when applied. This also make changes in CommitBear.py to pass AddNewlineAction when Result is yielded.
892e753
to
7ffaabe
Compare
Don't want to merge this until we have a green build here. coala-bears is already a bit messed up to add more changes. Will keep this back in my head though. |
This adds two new actions for
GitCommitBear
:AddNewlineAction
: This adds a newline between shortlog and body of the commit message.EditCommitMessageAction
: Open an editor in which user can edit the commit messageThis also modifies CommitBear.py to pass these actions when Result is yielded.
Temp changes for travis
commit is temporary commit because this PR depends on coala/coala#6029