Closed aberonni closed 5 years ago
I am not going to review it in details, but the first thing I noticed is that the man page git-open.1.md
is not updated and that there are no tests for this.
And #125 speaks about the possibility to add a hash. I don't see that in your PR.
I think I can add a test (I'll try) and also documentation on the man page.
Not sure I'm proficient enough with bash to (correctly) add the support for the hash. Could you suggest how to go about that? I think ideally:
# open current commit
git-open --commit
# open provided commit
git-open --commit 4eed00f
@ffes I've tried to address your feedback - please let me know what you think.
The only thing left is the possibility to specify a sha
. I'm not sure how best to do that so I'm thinking that it could be added in a separate PR without closing #125 - this change alone adds a useful functionality (and someone could build the other feature on top of it). What do you think?
With #143 merged, a conflict arised. Could you fix that please?
Other then that, it looks good to me.
@derimagia Could you take at this, please?
Note that this does not fix #125 (that also wants to be able to open a specific hash) but it looks like GitHub thinks it does, so #125 needs to be reopened when this is merged.
I've resolved the conflict and renamed PR and commit - possibly this will avoid automatically closing #125
This adds a
--commit
option that opens the current commit in the browser.I've only tested this with gitlab seeing as I needed it there. If this could be useful I can integrate any feedback you might have to get this proposal merged.