Re: How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Matt]

  On Sun, 17 Mar 2024 17:26:48 +0100  Max Nikulin  wrote --- 
 > On 17/03/2024 20:42, Matt wrote:
 > >    On Sun, 17 Mar 2024 11:31:00 +0100  Ihor Radchenko  wrote ---
 > >   >
 > >   > (1) revert the commit; (2) re-apply the
 > >   > commit version with the correct author attribution.
 > > 
 > > Done.
 > > 
 > > 1. git revert 
 > > 2. make changes (e.g. emacs  followed by *type-type-type* or some 
 > > incantation of 'git apply' or 'git am')
 > > 3. git commit --author "Arthur Author "
 > 
 > In this particular case "git am" works fine and no explicit "git commit" 
 > is necessary.

Thank you for taking the time to verify the patch formatting.  Sounds like a 
PEBKAC.  I'll look into fixing it :)

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode

Re: How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Max Nikulin]

On 17/03/2024 20:42, Matt wrote:

   On Sun, 17 Mar 2024 11:31:00 +0100  Ihor Radchenko  wrote ---
  >
  > (1) revert the commit; (2) re-apply the
  > commit version with the correct author attribution.

Done.

1. git revert 
2. make changes (e.g. emacs  followed by 
*type-type-type* or some incantation of 'git apply' or 'git am')
3. git commit --author "Arthur Author "


In this particular case "git am" works fine and no explicit "git commit" 
is necessary.

Re: How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Ihor Radchenko]

Matt  writes:

>  > We should avoid force pushing unless something is terribly broken.
>  > What you may do instead is (1) revert the commit; (2) re-apply the
>  > commit version with the correct author attribution.
>
> Done.
> ...
> I added a little section within copyright: 
> https://git.sr.ht/~bzg/worg/commit/80152bee771b755aedfbe488497c5e4d0e7457c2

Thanks!

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Matt]

  On Sun, 17 Mar 2024 11:31:00 +0100  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 >
 > > First, would you like me to update the commit?  If so, I will need 
 > > guidance.  The correct procedure to change the author after committing to 
 > > remote is unclear to me.  I would think it's something like sync my local 
 > > copy with the latest remote version, update the author locally, and force 
 > > push the change.  I would then expect that the next time someone pulls, it 
 > > would update their local with the author change.  It would, however, cause 
 > > a conflict, I think, for someone in the middle of making a change who has 
 > > not synced with the forced push version and is trying to push their change.
 > 
 > We should avoid force pushing unless something is terribly broken.
 > What you may do instead is (1) revert the commit; (2) re-apply the
 > commit version with the correct author attribution.

Done.

For the benefit of future me or anyone else who cares, I did:

1. git revert 
2. make changes (e.g. emacs  followed by 
*type-type-type* or some incantation of 'git apply' or 'git am')
3. git commit --author "Arthur Author "
4. git push

'git revert', in this case, basically swaps the plus and minus signs in the 
diff for the specified commit and submits that as a new set of changes.  After 
applying those changes, it's possible, in this case, to proceed with "what you 
should have done in the first place".

 > > Second, I can update Worg with an explanation that it's important to 
 > > credit authors using git's author field and how to do this.  Unless I 
 > > missed it, worg/org-contribute makes no mention of the author field.  The 
 > > version of git packaged by my distro is 2.41.0 and, AFAICT, has no -A flag 
 > > for 'git' or 'git commit'.  However, the following works on my machine 
 > > and, I guess, is the long option form:
 > >
 > > git commit --author "Arthur Override "
 > 
 > You are right. Looks like -A is just Magit shortcut.
 > 
 > As for crediting authors, we may document it in 
 > https://orgmode.org/worg/org-maintenance.html#copyright
 > Although, it is under "core maintainer" section. Maybe we can make a
 > dedicated section for maintainers on how to deal with patch submissions.

I added a little section within copyright: 
https://git.sr.ht/~bzg/worg/commit/80152bee771b755aedfbe488497c5e4d0e7457c2

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode

Re: How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Ihor Radchenko]

Matt  writes:

>  > For future, we prefer keeping the original commit author in the "author"
>  > field of the commits - this is important to keep track of the number of
>  > changed lines for contributors without FSF copyright assignment.
>  
> Thank you for letting me know this is an issue.
>
> First, would you like me to update the commit?  If so, I will need guidance.  
> The correct procedure to change the author after committing to remote is 
> unclear to me.  I would think it's something like sync my local copy with the 
> latest remote version, update the author locally, and force push the change.  
> I would then expect that the next time someone pulls, it would update their 
> local with the author change.  It would, however, cause a conflict, I think, 
> for someone in the middle of making a change who has not synced with the 
> forced push version and is trying to push their change.

We should avoid force pushing unless something is terribly broken.
What you may do instead is (1) revert the commit; (2) re-apply the
commit version with the correct author attribution.

> Second, I can update Worg with an explanation that it's important to credit 
> authors using git's author field and how to do this.  Unless I missed it, 
> worg/org-contribute makes no mention of the author field.  The version of git 
> packaged by my distro is 2.41.0 and, AFAICT, has no -A flag for 'git' or 'git 
> commit'.  However, the following works on my machine and, I guess, is the 
> long option form:
>
> git commit --author "Arthur Override "

You are right. Looks like -A is just Magit shortcut.

As for crediting authors, we may document it in 
https://orgmode.org/worg/org-maintenance.html#copyright
Although, it is under "core maintainer" section. Maybe we can make a
dedicated section for maintainers on how to deal with patch submissions.

> Third, this is at least the second time I've had issues working with a 
> diff/patch.  The reason I submitted the change the way I did is that I could 
> not get 'git apply ' to work.  I only got a useless error like 
> "error: corrupt patch at line 10".  It's not clear to me if this is an error 
> on my end or if the patch is indeed ill-formatted.  Can you confirm that the 
> submitted patch is well-formatted?

There are several types of patches that may need to be applied
differently. Plain "diff" patches can be applied using git apply, while
maildir/.patch patches can be applied using git am.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

How to properly attribute authorship with Git (was Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name)

[Matt]

  On Sat, 16 Mar 2024 11:11:38 +0100  Ihor Radchenko  wrote --- 

 > I notice that you changed the commit author and only referenced Aaron in
 > Submitted By: metadata.
 > 
 > For future, we prefer keeping the original commit author in the "author"
 > field of the commits - this is important to keep track of the number of
 > changed lines for contributors without FSF copyright assignment.
 
Thank you for letting me know this is an issue.

First, would you like me to update the commit?  If so, I will need guidance.  
The correct procedure to change the author after committing to remote is 
unclear to me.  I would think it's something like sync my local copy with the 
latest remote version, update the author locally, and force push the change.  I 
would then expect that the next time someone pulls, it would update their local 
with the author change.  It would, however, cause a conflict, I think, for 
someone in the middle of making a change who has not synced with the forced 
push version and is trying to push their change.

Second, I can update Worg with an explanation that it's important to credit 
authors using git's author field and how to do this.  Unless I missed it, 
worg/org-contribute makes no mention of the author field.  The version of git 
packaged by my distro is 2.41.0 and, AFAICT, has no -A flag for 'git' or 'git 
commit'.  However, the following works on my machine and, I guess, is the long 
option form:

git commit --author "Arthur Override "

Third, this is at least the second time I've had issues working with a 
diff/patch.  The reason I submitted the change the way I did is that I could 
not get 'git apply ' to work.  I only got a useless error like 
"error: corrupt patch at line 10".  It's not clear to me if this is an error on 
my end or if the patch is indeed ill-formatted.  Can you confirm that the 
submitted patch is well-formatted?

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode

Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name

[Ihor Radchenko]

Matt  writes:

> Finally had time to commit it. 
>
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0e2a9524dc6da8b4d60672e85aba74076baac211

Thanks!

I notice that you changed the commit author and only referenced Aaron in
Submitted By: metadata.

For future, we prefer keeping the original commit author in the "author"
field of the commits - this is important to keep track of the number of
changed lines for contributors without FSF copyright assignment.

You may consider using https://git.kyleam.com/piem/about/ to apply
patches submitted as emails automatically. Or you can assign the commit
author manually, using -A git commit switch (magit also provides a
transient menu option for this).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name

[Aaron Zeng]

Thanks for the merge!

On Fri, Mar 15, 2024, at 17:38, Matt wrote:
> Finally had time to commit it. 
>
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0e2a9524dc6da8b4d60672e85aba74076baac211
>
> Nice catch and thanks for your contribution!
>
> --
> Matt Trzcinski
> Emacs Org contributor (ob-shell)
> Learn more about Org mode at https://orgmode.org
> Support Org development at https://liberapay.com/org-mode

Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name

[Matt]

Finally had time to commit it. 

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0e2a9524dc6da8b4d60672e85aba74076baac211

Nice catch and thanks for your contribution!

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode

Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name

[Ihor Radchenko]

Matt  writes:

> Thank you for your report and the patch. 

Matthew, you are free to apply patches relevant to ob-shell to savannah
repository, if you think that they are fine.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] lisp/ob-shell.el: Also override explicit-shell-file-name

[Matt]

  On Mon, 11 Mar 2024 06:12:47 +0100  Aaron L. Zeng  wrote --- 
 > * lisp/ob-shell.el (org-babel-shell-initialize): Override
 > explicit-shell-file-name in addition to shell-file-name.
 > 
 > When a session with shell source blocks, execution calls `shell',
 > which checks `explicit-shell-file-name' variable before
 > `shell-file-name', to determine what shell to run.  If the user has
 > customized this variable to affect the behavior of M-x shell,
 > `org-babel-shell-initialize' should still run the shell specified by
 > the org source block's language name.
 > 
 > TINYCHANGE
 > ---
 >  lisp/ob-shell.el | 3 ++-
 >  1 file changed, 2 insertions(+), 1 deletion(-)
 > 
 > diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
 > index 551c3785d..35d9e9376 100644
 > --- a/lisp/ob-shell.el
 > +++ b/lisp/ob-shell.el
 > @@ -81,7 +81,8 @@ is modified outside the Customize interface."
 >  (lambda (body params)
 >    (:documentation
 > (format "Execute a block of %s commands with Babel." name))
 > -  (let ((shell-file-name name))
 > +  (let ((explicit-shell-file-name name)
 > +(shell-file-name name))
 >  (org-babel-execute:shell body params
 >(put fname 'definition-name 'org-babel-shell-initialize))
 >  (defalias (intern (concat "org-babel-variable-assignments:" name))
 > -- 
 > 2.42.0

Thank you for your report and the patch. 

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode