Conversation
2fa1b4b to
cb21b13
Compare
cb21b13 to
9bbed4f
Compare
benthayer
left a comment
There was a problem hiding this comment.
I'm assuming that adding branches would change the method signature.
To avoid a headache, we can implement that before merging, but if the headache of refactoring later is acceptable, this functionality is still useful and can be merged after the other issues are addressed
| file_operator = operations.get_operator() | ||
| display_data = [ | ||
| {"_name": "Before", "_commit": file_operator.repo.commit(before_ref)}, | ||
| {"_name": "After", "_commit": file_operator.repo.commit(after_ref)} |
There was a problem hiding this comment.
Here, it might be worth it to do tuples or nested dictionaries. Using underscores is a bit weird here.
| ) | ||
|
|
||
|
|
||
| def amending_message(before_ref, after_ref, show_hashes=True, show_files=True, show_refs=True): # noqa: E501 |
There was a problem hiding this comment.
Method doesn't let you show the branches as shown in #257
|
@benthayer The branch displaying support is actually from #310 -- I took the code from there and implemented it here, but now that I think of it, we would have different commits with the same "content". If you're okay with it, I can remove the branch-displaying code from these commits, work on everything else not including the branches, and then rebase on master when #310 is merged. What do you think? |
Fixes #264
Tested locally on skill 2-1