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

Button to add co-authors while committing #334

Closed
LinqLover opened this issue Jul 14, 2021 · 14 comments
Closed

Button to add co-authors while committing #334

LinqLover opened this issue Jul 14, 2021 · 14 comments

Comments

@LinqLover
Copy link
Contributor

This should be integrated into Squot: https://github.com/ShirleyNekoDev/Squot-CoAuthors

It's been 2 years since I have been using it, but it was a helpful tool for me.

If @ShirleyNekoDev and @LeonBein would agree with MIT licensing of this repository, we could integrate this extension into Squot.

@LeonBein
Copy link
Contributor

Definitely fine by me, I think the original code was by @ShirleyNekoDev.
@ShirleyNekoDev, what do you say?

@j4yk
Copy link
Collaborator

j4yk commented Jul 16, 2021

Just quickly looked at the code. Do I understand correctly that it adds a button to the save dialog which when clicked prompts you to select the co-authors, then these will be appended to the commit message (which can then still be edited)?

If that is it, nice. My only concern would be the precious space that the button might take up. Do you have a screenshot for me?

What is the use case? Pair programming attribution? Because otherwise if the methods come from various authors, why not let them make separate commits?

@LinqLover
Copy link
Contributor Author

What is the use case? Pair programming attribution?

Exactly, at least this was my main use case. 🙂

My only concern would be the precious space that the button might take up. Do you have a screenshot for me?

Unfortunately not right now from my phone, but the button was way too large, yes. This is probably something which we would like to change. Maybe we could insert a menu item into the text pane for the commit message instead?

@LinqLover
Copy link
Contributor Author

image

@j4yk
Copy link
Collaborator

j4yk commented Jul 16, 2021

If we could add a 60 character per line limitation to the text editor, having buttons on the right could actually make sense. Menu item also sounds good to me.

@LinqLover
Copy link
Contributor Author

If we could add a 60 character per line limitation to the text editor

Do we want that? I really think this should be a responsibility of tooling these days. I'm not really a big fan of pictures like this: :-)

image

But yes, there might be some other pre-commit actions, maybe to create an overview of slips in the working copy ...

@j4yk
Copy link
Collaborator

j4yk commented Jul 16, 2021

In the Smalltalk tools, limiting to 60 chars does not make much sense. But since this is also about Git, one should also mind the conventions formed by console users. For the headline in commit messages it also makes sense due to the limited space in commit lists of all kinds. Eclipse has the nice solution of automatically and responsively inserting line breaks, so if you edit the middle of the sentence, the the words move around the line breaks as well. But it is actually off-topic in this issue here.

@LinqLover
Copy link
Contributor Author

Off-topic, but still interesting. :-) I agree that the headline should not be too long - looks strange in the Squit Browser, too - but for the rest of the messages, I actually don't see why git on the CLI could not wrap the message by itself. Removing the linebreaks at a later point in time is much harder than inserting them IMHO.

@ShirleyNekoDev
Copy link

I've added the MIT license to my repository.
Glad you found my tool useful and want to develop it into a full feature! Feel free to integrate or modify the code for your convenience :)

j4yk added a commit that referenced this issue Dec 17, 2021
@j4yk
Copy link
Collaborator

j4yk commented Dec 17, 2021

Merged the other repository, merged the copyright into the LICENSE file and merged the former README.md into the main README.md.

My only concern is that it is not very discoverable in the image how to modify the authors list. Any suggestions?

j4yk added a commit that referenced this issue Dec 17, 2021
@LinqLover
Copy link
Contributor Author

Very nice, thank you!

My only concern is that it is not very discoverable in the image how to modify the authors list. Any suggestions?

Add a "add new co-author" item to the top/bottom of the list, and when it is chosen, ask the user for name and e-mail address in two dialogs (or a single merged one)? :-)

j4yk added a commit that referenced this issue Dec 30, 2021
@j4yk
Copy link
Collaborator

j4yk commented Dec 30, 2021

Add a "add new co-author" item to the top/bottom of the list, and when it is chosen, ask the user for name and e-mail address in two dialogs (or a single merged one)? :-)

I added an item to just open the browser on the method. Please check it out.

@j4yk j4yk added this to the release 0.10.0 milestone Jan 22, 2022
@j4yk
Copy link
Collaborator

j4yk commented Jul 3, 2022

Since I have not received any further feedback, I will assume that it is fine as it is.

@j4yk j4yk closed this as completed Jul 3, 2022
@LinqLover
Copy link
Contributor Author

Thank you! 🎉

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

No branches or pull requests

4 participants