[v2,1/5] manual: Update to mention ENODEV for ttyname and ttyname_r

Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
introduced ENODEV as a possible error condition for ttyname and ttyname_r.
The manual should mention this.

v2:
 - Fix typo: psuedo->pseudo
 - Improve wording
 - Fix ChangeLog item name
---
 ChangeLog            | 5 +++++
 manual/terminal.texi | 5 +++++
 2 files changed, 10 insertions(+)

On Thu, Nov 02, 2017 at 02:53:42PM -0400, Luke Shumaker wrote:
> Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
> introduced ENODEV as a possible error condition for ttyname and ttyname_r.
> The manual should mention this.
> 
> v2:
>  - Fix typo: psuedo->pseudo
>  - Improve wording
>  - Fix ChangeLog item name
> ---
>  ChangeLog            | 5 +++++
>  manual/terminal.texi | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 0125305f6e..f6137669bf 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-11-02  Luke Shumaker  <lukeshu@parabola.nu>
> +
> +	* manual/terminal.texi (Is It a Terminal):
> +	Mention ENODEV for ttyname and ttyname_r.
> +
>  2017-11-02  Mike FABIAN  <mfabian@redhat.com>
>  
>  	[BZ #22382]
> diff --git a/manual/terminal.texi b/manual/terminal.texi
> index 4fef5045b8..4aace48b14 100644
> --- a/manual/terminal.texi
> +++ b/manual/terminal.texi
> @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
>  @item ERANGE
>  The buffer length @var{len} is too small to store the string to be
>  returned.
> +
> +@item ENODEV
> +The @var{filedes} is associated with a terminal device that is a slave
> +pseudo-terminal, but the file name associated with that device could
> +not be determined.  This is a GNU extension.
>  @end table
>  @end deftypefun

Reviewed-By: Christian Brauner <christian.brauner@ubuntu.com>

>  
> -- 
> 2.15.0
>

On Thu, Nov 02, 2017 at 02:53:42PM -0400, Luke Shumaker wrote:
> Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
> introduced ENODEV as a possible error condition for ttyname and ttyname_r.
> The manual should mention this.

This short abbreviated form of commit names may become ambiguous in the
future, let's use something less ambiguous, or just the full commit name.
 
Unless the referenced commit lacks a proper attribution, additional
attribution that duplicates the information in the referenced commit
is redundant.

A commit message looks inconsistent when it says that A should be changed
while the commit implements the change.

Summarising, I'd expect a commit message like this:

Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced ENODEV
as a possible error condition for ttyname and ttyname_r.
Update the manual to mention this GNU extension.
 
> v2:
>  - Fix typo: psuedo->pseudo
>  - Improve wording
>  - Fix ChangeLog item name
> ---
>  ChangeLog            | 5 +++++
>  manual/terminal.texi | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 0125305f6e..f6137669bf 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-11-02  Luke Shumaker  <lukeshu@parabola.nu>
> +
> +	* manual/terminal.texi (Is It a Terminal):
> +	Mention ENODEV for ttyname and ttyname_r.
> +
>  2017-11-02  Mike FABIAN  <mfabian@redhat.com>
>  
>  	[BZ #22382]
> diff --git a/manual/terminal.texi b/manual/terminal.texi
> index 4fef5045b8..4aace48b14 100644
> --- a/manual/terminal.texi
> +++ b/manual/terminal.texi
> @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
>  @item ERANGE
>  The buffer length @var{len} is too small to store the string to be
>  returned.
> +
> +@item ENODEV
> +The @var{filedes} is associated with a terminal device that is a slave
> +pseudo-terminal, but the file name associated with that device could
> +not be determined.  This is a GNU extension.
>  @end table
>  @end deftypefun

The change itself is OK, thanks for updating documentation.

Reviewed-by: Dmitry V. Levin <ldv@altlinux.org>

On Mon, Nov 06, 2017 at 07:09:43PM +0300, Dmitry V. Levin wrote:
> On Thu, Nov 02, 2017 at 02:53:42PM -0400, Luke Shumaker wrote:
> > Commit 15e9a4f3 by Christian Brauner <christian.brauner@canonical.com>
> > introduced ENODEV as a possible error condition for ttyname and ttyname_r.
> > The manual should mention this.
> 
> This short abbreviated form of commit names may become ambiguous in the
> future, let's use something less ambiguous, or just the full commit name.
>  
> Unless the referenced commit lacks a proper attribution, additional
> attribution that duplicates the information in the referenced commit
> is redundant.
> 
> A commit message looks inconsistent when it says that A should be changed
> while the commit implements the change.
> 
> Summarising, I'd expect a commit message like this:
> 
> Commit 15e9a4f378c8607c2ae1aa465436af4321db0e23 introduced ENODEV
> as a possible error condition for ttyname and ttyname_r.
> Update the manual to mention this GNU extension.
>  
> > v2:
> >  - Fix typo: psuedo->pseudo
> >  - Improve wording
> >  - Fix ChangeLog item name
> > ---
> >  ChangeLog            | 5 +++++
> >  manual/terminal.texi | 5 +++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/ChangeLog b/ChangeLog
> > index 0125305f6e..f6137669bf 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2017-11-02  Luke Shumaker  <lukeshu@parabola.nu>
> > +
> > +	* manual/terminal.texi (Is It a Terminal):
> > +	Mention ENODEV for ttyname and ttyname_r.
> > +
> >  2017-11-02  Mike FABIAN  <mfabian@redhat.com>
> >  
> >  	[BZ #22382]
> > diff --git a/manual/terminal.texi b/manual/terminal.texi
> > index 4fef5045b8..4aace48b14 100644
> > --- a/manual/terminal.texi
> > +++ b/manual/terminal.texi
> > @@ -109,6 +109,11 @@ The @var{filedes} is not associated with a terminal.
> >  @item ERANGE
> >  The buffer length @var{len} is too small to store the string to be
> >  returned.
> > +
> > +@item ENODEV
> > +The @var{filedes} is associated with a terminal device that is a slave
> > +pseudo-terminal, but the file name associated with that device could
> > +not be determined.  This is a GNU extension.
> >  @end table
> >  @end deftypefun
> 
> The change itself is OK, thanks for updating documentation.
> 
> Reviewed-by: Dmitry V. Levin <ldv@altlinux.org>

I'm going to change the wording in some of the commit messages anyway so I would
just do this substitution when I apply, i.e. if people are fine with that.

> 
> 
> -- 
> ldv

On Mon, Nov 06, 2017 at 07:19:46PM +0100, Christian Brauner wrote:
[...]
> I'm going to change the wording in some of the commit messages anyway so I would
> just do this substitution when I apply, i.e. if people are fine with that.

As commit messages are part of commits, they are also subject of review.

On Mon, Nov 06, 2017 at 10:56:28PM +0300, Dmitry V. Levin wrote:
> On Mon, Nov 06, 2017 at 07:19:46PM +0100, Christian Brauner wrote:
> [...]
> > I'm going to change the wording in some of the commit messages anyway so I would
> > just do this substitution when I apply, i.e. if people are fine with that.
> 
> As commit messages are part of commits, they are also subject of review.

Thanks. Good to know. I've just seen commit messages being changed when applying
patches to master that's why I assumed grammatical and syntactial errors can be
fixed by the committer.

On Mon, Nov 06, 2017 at 10:18:01PM +0100, Christian Brauner wrote:
> On Mon, Nov 06, 2017 at 10:56:28PM +0300, Dmitry V. Levin wrote:
> > On Mon, Nov 06, 2017 at 07:19:46PM +0100, Christian Brauner wrote:
> > [...]
> > > I'm going to change the wording in some of the commit messages anyway so I would
> > > just do this substitution when I apply, i.e. if people are fine with that.
> > 
> > As commit messages are part of commits, they are also subject of review.
> 
> Thanks. Good to know. I've just seen commit messages being changed when applying
> patches to master that's why I assumed grammatical and syntactial errors can be
> fixed by the committer.

Trivial fixes of spelling or grammatical errors do not require a review.