Skip to content

Conversation

@PrasadBabarendaGamage
Copy link
Contributor

Adding updates required to address issue #65 (includes updates from Richard's review of pull request #66)

Updating developer documentation based on Richard's review comments from pull request 66. 

A few more updates are required.

Addresses issue OpenCMISS#65
@PrasadBabarendaGamage
Copy link
Contributor Author

Not all of Richards reviewer comments from pull request #66 have been resolved - work in progress.

@hsorby
Copy link
Contributor

hsorby commented May 7, 2017

What's with the manual line wrapping?

Also adds documentation to let other developers know not to manually wrap sphinx documentation.

Addresses issue OpenCMISS#65
Addresses issue OpenCMISS#65
===================

The ``git add`` and ``git commit`` commands should be obvious, the ``git push`` command sets the local branch ``issue46`` to be linked with the remote branch ``issue46`` in the origin (the default shorthand for your libCellML repository on GitHub) repository, this branch will be created in the origin repository if it doesn't already exist.
From there create a pull request from your feature branch to the :term:`prime repository` develop branch. When creating the pull request make sure to add in the comment ``addresses issue #46`` (replace the number 46 with the actual number of the issue you are addressing), or something to that effect. This will create a link between the issue and the pull request enabling other people to see that you are working on this issue and comment on your work.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nickerso
Should addresses issue #46 also be used when making commits (not just a pull request), to make sure the commit you are making is linked to an issue? This is not mentioned in the "Committing changes to your feature branch" section.

Copy link
Contributor

Choose a reason for hiding this comment

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

commits on the same branch will automatically update the pull request, so I don't see a need for mentioning it in every commit. If referencing a different issue, then it makes sense to reference it - but its enough to have #46 in whatever context it needs to be. Its also trivial to reference issues in other repos when required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Andre, committing to the same branch without mentioning the issue in the commit message would update the pull. However, the issue itself would not get updated - this seems important as people won't know what work has been done if they only browse the issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a show stopper for me, either way.

Finished updating developer documentation based on Richard's review comments from pull request 66.

Addresses issue OpenCMISS#65
@PrasadBabarendaGamage
Copy link
Contributor Author

Hi all, this pull request is ready. It addresses the issues Richard mentioned in pull request #66 (I've made some comments there regarding the updates). Items that have not been addressed have been added to new issues. If you have any suggestions for major improvements then please update the appropriate issues/add new issues. Please also indicate priority in the issue. Due to time constraints, I unfortunately will only be able to work on high priority issues. Will aim to work on adding some tutorials next based on e.g. Andre's monodomain example, and add documentation to help others do (see issue #67 for more information).

@PrasadBabarendaGamage
Copy link
Contributor Author

@hsorby @nickerso @chrispbradley Hi all, hoping if we could please merge this pull request in as soon as possible. I can then point the developers to the staging site to have a look. We created a list of who is working on what in OpenCMISS and there are columns for issues and pull requests. Would be great if developers can see what is the recommend workflow for adding these items. This pull request provides the latest info for explaining how to do this.

@PrasadBabarendaGamage
Copy link
Contributor Author

@hsorby @nickerso @chrispbradley Hi guys, Andy just mentioned that the Stuttgart developers are having a developer meeting tomorrow and he was going to mention the developer docs - it would be great if this could be pulled in and up on staging by then so that they can have a look and comment/provide feedback.
I think in future it might be best if we can just push to staging prior to review so that reviewers can compare with the non-staging site. We then also have a working link to the latest docs on staging.

Thanks

Copy link
Contributor

@nickerso nickerso left a comment

Choose a reason for hiding this comment

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

Good work addressing all the comments @PrasadBabarendaGamage. It all looks good to me and should be merged in.

@hsorby hsorby merged commit d2da94d into OpenCMISS:develop Jun 29, 2017
@PrasadBabarendaGamage
Copy link
Contributor Author

@nickerso @hsorby Thanks a lot guys!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants