Skip to content
This repository was archived by the owner on Jan 22, 2019. It is now read-only.

Development process documentation specifically about contribution #44

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open

Development process documentation specifically about contribution #44

wants to merge 21 commits into from

Conversation

codecurve
Copy link
Contributor

For #43.

As agreed at CellML meeting a few weeks ago, we will restart
the documentation of the LibCellML process using the
MAP client dev process docs as a starting point.

This is one of the docs from a family of related docs.  Some of
the others will probably also be used.
Randall Britten added 10 commits March 9, 2015 11:00
Mostly just done by search and replace.
Etc =
- Captured that contribution can be work other than coding
- BuildBot triggered on push
Main difference: no sub repos.
Also: separated out test driven discussion (since not all
work is coding (e.g. docs).
Again, main difference: no sub-repos at this stage.
The diagram can be edited using Google Drive - Drawing, make
a copy of the drawing that is at this URL: http://goo.gl/cJFBGf
As discussed at today's meeting, we all agreed that it makes more
sense to keep the issue open, since sometimes the first pull
request does not get merged, and multiple pull requests constitute
the work done to resolve an issue.

Also, the posting of comments requesting reviews was added.

Also, some other rewording.
@nickerso
Copy link
Contributor

do we know why the build is failing?

@codecurve
Copy link
Contributor Author

The Windows 8 test run fails the test (there is only 1 test). This branch is based off the head of develop, which passed last time on all platforms (but see #45). The changes don't affect code, so not sure why build is failing. It builds and passes the test on my Windows 8 machine, so I think it is just the BuildBot playing-up.


Before you begin you will need to have a few prerequisites satisfied:

#. GitHub user account (if necessary, create one using this link https://github.com/join ) (for the rest of this document we will call our user *bob*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to go with a demonstration username that is unlikely to ever exist? libCellML-developer maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, see 0722326

@nickerso
Copy link
Contributor

definition of prime repo should come before it is used in the docs. Perhaps using a glossary would help with such repeated terms.

Randall Britten added 2 commits March 27, 2015 17:46
Main change is due to not using submodules on libCellML.
@codecurve
Copy link
Contributor Author

Regarding use of term "prime repo", I've opted to follow what @hsorby has done for the MAP docs, and defined "prime" in the section intro (in fact I used the MAP section intro as the starting point).

While I agree that a glossary is a good idea, I think that is a separate task. See 05327ba

@nickerso
Copy link
Contributor

A glossary is trivial to add and without defining the prime repository in the contribution document (which is the subject of this pull request?) it isn't a self-contained document. So potential contributors will be required to read the full developer documentation in order to work things out. I'd really recommend adding a glossary now, simply defining the prime repository at this point.

Just to help, here is how to create a glossary: http://sphinx-doc.org/markup/para.html#glossary

@codecurve
Copy link
Contributor Author

@nickerso, it seems you perhaps missed 05327ba , which defines “prime repository” before the term’s first usage.

This means that there is no reason remaining to not merge the pull request.

Continually tacking on “trivial” tasks to pull requests drags them out.

A glossary is trivial to add and without defining the prime repository in the contribution document (which is the subject of this pull request?) it isn't a self-contained document. So potential contributors will be required to read the full developer documentation in order to work things out. I'd really recommend adding a glossary now, simply defining the prime repository at this point.

Just to help, here is how to create a glossary: http://sphinx-doc.org/markup/para.html#glossary

@nickerso
Copy link
Contributor

nickerso commented Apr 1, 2015

I thought I was pretty clear in my comment - but to be explicit, yes, I have seen 05327ba.

In order to break up the task of writing the complete developer documentation (as per milestone 0 in the libCellML roadmap) you have split the task up into separate chunks - which is fine. But, to me, this requires that each such chunk is understandable on its own. In this case, it is not because the reader is required to revert to the incomplete higher level documentation to understand what a prime repository is.

Addressing reviewer feedback necessitates taking longer to complete a task but will hopefully make for a better project overall and a tool that developers will want to use and contribute to. Saying that my feedback is dragging out the process seems an unhelpful way in which to try and grow a community sprit and encourage reviewers to volunteer their time to the libCellML project.

@codecurve
Copy link
Contributor Author

The documentation under this pull request is self contained, since it defines the prime repo.
I think the discouragement of contribution goes both ways, since making it burdensome to get pull requests merged will probably also have the same effect.

Randall Britten and others added 3 commits April 1, 2015 16:13
Thus, noted that the interface of the implementation can
be included in the test commit in order for compilation
to succeed.
@nickerso
Copy link
Contributor

nickerso commented Apr 1, 2015

I have just created a pull request against Randall's fork which adds in the glossary - since I really do believe it is necessary in this case. I also noticed that the development process index document was not included anywhere - so turns out 05327ba didn't actually address the issue of defining the term "prime repository". I have also fixed this in my pull request.

If Randall is happy to merge that in, it will clear up at least that particular issue I have with this pull request. I'm still digesting the impact of 0af85a0.

@nickerso
Copy link
Contributor

nickerso commented Apr 1, 2015

oops, forgot the link: codecurve/libcellml#9

Development process documentation updates
@codecurve
Copy link
Contributor Author

Thanks Andre, good job. It has been merged into my branch.

I have just created a pull request against Randall's fork which adds in the glossary - since I really do believe it is necessary in this case. I also noticed that the development process index document was not included anywhere - so turns out 05327bahttps://github.com/cellml/libcellml/commit/05327babf25e46821031288d33f5966f0225895a didn't actually address the issue of defining the term "prime repository". I have also fixed this in my pull request.

If Randall is happy to merge that in, it will clear up at least that particular issue I have with this pull request. I'm still digesting the impact of 0af85a0cellml/libcellml@0af85a0.

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

Successfully merging this pull request may close these issues.

3 participants