GSOC: Introduction

[Dmitry V. Levin]

On Sun, Mar 05, 2017 at 06:27:49PM +0530, Abhishek Tiwari wrote:
> On Sun, Mar 5, 2017 at 8:48 AM, Dmitry V. Levin wrote:
> > On Sun, Mar 05, 2017 at 01:54:33AM +0530, Abhishek Tiwari wrote:
> >> On Sat, Mar 4, 2017 at 5:41 PM, Abhishek Tiwari wrote:
> >> > Good evening everyone,
> >> > I am Abhishek Tiwari pursuing M.Tech IIT Kharagpur. I am looking
> >> > forward to contribute to strace Project Idea -> Netlink socket
> >> > parsers.
> >> >
> >> > Thank you.
> >>
> >> I have made a change in INSTALL file (there the bootstrap script was
> >> not mentioned to be run in installation step). Please review the
> >> patch.
> >
> > This INSTALL file comes directly from GNU Automake, it's intended
> > for those who build the package from tarball.
> >
> > To build strace "from git", one has to run ./bootstrap script first.
> > This is mentioned in README-hacking, but not in INSTALL file,
> > which is somewhat confusing.
> >
> > Perhaps it would be better to create a separate file with instructions how
> > to build strace "from git" and reference INSTALL from there.
> >
> > Alternatively, we can modify INSTALL file, but the new wording shouldn't
> > be confusing to those who build strace from tarball.
> >
> >> Is sending patch preferred over sending pull request or I can directly
> >> send pull requests from github?
> >
> > We prefer patches sent to this mailing list.
> 
> Thank you ldv for your response
> I have made changes to README.md file so that instruction would be
> visible as anyone visits the repository on Github.
> Below is patch attached for the same.

Adding a reference in README.md file is a good idea.
At the same time, mentioning ./bootstrap is not enough.
For example, README-hacking that mentions ./bootstrap also describes
its requirements.

I think it would be better if all this "how to build from git" information
was moved into a separate file, which could be referenced by README.md and
README-hacking.

See my technical comments below.

> --- a/README.md
> +++ b/README.md
> @@ -7,3 +7,16 @@ See the file [NEWS](https://raw.githubusercontent.com/strace/strace/master/NEWS)
>  Please send bug reports and enhancements to [the strace mailing list](https://lists.sourceforge.net/lists/listinfo/strace-devel).
>  
>  [![Build Status](https://travis-ci.org/strace/strace.svg?branch=master)](https://travis-ci.org/strace/strace) [![Code Coverage](https://codecov.io/github/strace/strace/coverage.svg?branch=master)](https://codecov.io/github/strace/strace?branch=master)
> +
> +INSTALLATION GUIDELINES 
> +========================
> +
> +1. 	Install using tarball
> +--------------------------
> +
> +To install `strace` using tarball run GNU Automake command and follow instructions given in `INSTALL` file.
> +
> +2.	Build using git 
> +---------------------
> +
> +To build strace source code downloaded from git repository, first run `./bootstrap` script and then follow the guidelines given in `INSTALL` file.

1. This patch has whitespace issues and therefore doesn't apply:

Applying: Updated README.md file for building strace source code from git version
.git/rebase-apply/patch:14: trailing whitespace.
INSTALLATION GUIDELINES 
.git/rebase-apply/patch:22: trailing whitespace.
2.	Build using git 
fatal: 2 lines add whitespace errors.

2. The reference to Automake is wrong, a user is not expected to run any
automake commands just to build strace.

3. Assuming that INSTALL-git exists, the whole change could be reduced
to just a reference and placed before "send bug reports", e.g.

Please read the file [INSTALL-git](INSTALL-git) for installation instructions.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170305/6c61e25d/attachment.bin>