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

New actions for GitCommitBear #2927

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akshatkarani
Copy link
Member

@akshatkarani akshatkarani commented Jun 19, 2019

This adds two new actions for GitCommitBear:

  1. AddNewlineAction: This adds a newline between shortlog and body of the commit message.
  2. EditCommitMessageAction: Open an editor in which user can edit the commit message

This 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

@akshatkarani akshatkarani changed the title AddNewlineAction.py: A new action for GitCommitBear AddNewlineAction: A new action for GitCommitBear Jun 19, 2019
Add (N)ewline
"""
new_commit_message = '{}\n\n{}'.format(self.shortlog, self.body)
command = 'git commit --amend -m "{}"'.format(new_commit_message)
Copy link
Member Author

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?

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

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 👍

Copy link
Member Author

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.

Copy link
Member Author

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.

bears/vcs/CommitBear.py Outdated Show resolved Hide resolved
bears/vcs/CommitBear.py Outdated Show resolved Hide resolved
bears/vcs/CommitBear.py Outdated Show resolved Hide resolved
@akshatkarani akshatkarani force-pushed the add-newline-action branch 2 times, most recently from 775f5d0 to 31fff83 Compare June 20, 2019 15:06
@akshatkarani akshatkarani force-pushed the add-newline-action branch 3 times, most recently from 8ecf188 to b17afae Compare June 25, 2019 14:26
@akshatkarani akshatkarani changed the title AddNewlineAction: A new action for GitCommitBear New actions for GitCommitBear Jun 25, 2019
@akshatkarani akshatkarani force-pushed the add-newline-action branch 3 times, most recently from 51e9f5e to bd53589 Compare June 29, 2019 11:24
@akshatkarani
Copy link
Member Author

EditCommitMessageAction and AddNewlineAction in action

original_file_dict,
file_diff_dict,
applied_actions=()):
new_message, _ = run_shell_command('git log -1 --pretty=%B')
Copy link
Member Author

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.

@akshatkarani
Copy link
Member Author

Travis is failing because it depends on coala/coala#6029

@abhishalya
Copy link
Member

Yeah, please add the coala requirement as your branch.

@abhishalya
Copy link
Member

Yeah, please add the coala requirement as your branch.

Btw this is only temporary, to check if the changes work fine.

@akshatkarani
Copy link
Member Author

akshatkarani commented Jul 21, 2019

Yeah, please add the coala requirement as your branch.

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.

@abhishalya
Copy link
Member

You can ignore the first job of Sentinel since it fails for moban. But, the rest should pass... Looks like a breaking test.

@abhishalya
Copy link
Member

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.

@akshatkarani
Copy link
Member Author

Look at the error of the second build. This is because the changes in coala/coala#6029 are not there yet.

@abhishalya
Copy link
Member

coala is being installed correctly there from your branch, let me try this out locally

@abhishalya
Copy link
Member

@akshatkarani This seems to be the problem...

@akshatkarani akshatkarani force-pushed the add-newline-action branch 2 times, most recently from f567bff to b15ec55 Compare July 21, 2019 16:35
@akshatkarani
Copy link
Member Author

Removed the first sentinel build, now only one build is failing and the error is the same.

@akshatkarani
Copy link
Member Author

Okay build is passing with these temporary changes.

@abhishalya
Copy link
Member

Nice, (since other mentors aren't active) give me some time to check this (and the core changes) in more detail.

@abhishalya
Copy link
Member

Ping @Makman2

@Makman2
Copy link
Member

Makman2 commented Aug 13, 2019

  1. What about the temp-commit? ;)
  2. I would expect in the commit for AddNewlineAction to just do stuff with this action only, not mixing with EditCommitMessageAction. Please properly separate into the commits 👍

@akshatkarani
Copy link
Member Author

  1. What about the temp-commit? ;)

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.

  1. I would expect in the commit for AddNewlineAction to just do stuff with this action only, not mixing with EditCommitMessageAction. Please properly separate into the commits +1

Yes, I will.

@akshatkarani akshatkarani force-pushed the add-newline-action branch 3 times, most recently from 8b28fcc to 675cc1e Compare August 13, 2019 15:25
@akshatkarani
Copy link
Member Author

akshatkarani commented Aug 14, 2019

@Makman2 I have separated the commits properly.
Also some comments you made on commits have been lost because I amended them, link to it

@abhishalya
Copy link
Member

Temp commit not required now, also needs rebase.

@akshatkarani
Copy link
Member Author

Temp commit not required now

This PR still depends on coala/coala#6029 😕

@abhishalya
Copy link
Member

#2855 seems related to moban sync. Also, the commit should be a part of separate PR.

@akshatkarani
Copy link
Member Author

#2855 seems related to moban sync.

Looks pretty old, idk what the rest of changes are for.

Also, the commit should be a part of separate PR.

Okay, #2957

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.
@abhishalya
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

None yet

4 participants