www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Questions about Pull-Requests

reply "Benjamin Thaut" <code benjamin-thaut.de> writes:
Are there some guidelines for doing pull request?
Something like:
-Guidelines for commit messages
-TODO list before doing a pull request (e.g. run tests, 
untabifiy, etc)
-How to reproduce autotester issues
-General coding guidlines (what to indent, what not, line breaks 
etc)

Problems I'm currently having (with the latest git version):
-Running druntime unittests when druntime is compiled with -debug 
-g will produce huge masses of console output
-Running druntime unittests x86 windows will give me an 
assertion: core/time.d line 720 "seconds"
-Running druntime unittest x64 windows compiled with -g insteand 
of -O -release gives me an assertion in gc.gcx line 3200

Why does the autotester not find the unittest failure on x86 
windows?
Does the autotester only run the tests with release binaries? 
Because it misses the assertion when compiled with x64 windows 
debug.

Kind Regards
Benjamin Thaut
Dec 27 2012
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, December 27, 2012 11:46:38 Benjamin Thaut wrote:
 Are there some guidelines for doing pull request?
 Something like:
 -Guidelines for commit messages
If you're fixing a bug, you need to have something along the lines of "fix issue# 777" in it. Beyond that, I don't think that there are any actual guidelines. Just be intelligent about them.
 -TODO list before doing a pull request (e.g. run tests,
 untabifiy, etc)
Definitely run the unit tests on your local box first. It's kind of pointless to create a pull request that's guaranteed to fail the pull tester. Tabs aren't permitted, so make sure that you remove them if you've added them, but that's part of the general style guide.
 -How to reproduce autotester issues
Build them on the same type of system that's failing? I'm not sure what else you'd be looking for here.
 -General coding guidlines (what to indent, what not, line breaks
 etc)
This is mostly up-to-date now: http://dlang.org/dstyle.html Most of the guidelines relate to the API rather than code formatting. The main formatting ones are that spaces must be used (never tabs), braces should generally go on their own line (so Allman style, not K&R style), and lines have a soft limit of 80 characters and a hard limit of 120. Beyond that, formatting is pretty much just a question of trying to keep the style within a file consistent, but it varies from file to file.
 Problems I'm currently having (with the latest git version):
 -Running druntime unittests when druntime is compiled with -debug
 -g will produce huge masses of console output
 -Running druntime unittests x86 windows will give me an
 assertion: core/time.d line 720 "seconds"
 -Running druntime unittest x64 windows compiled with -g insteand
 of -O -release gives me an assertion in gc.gcx line 3200
 
 Why does the autotester not find the unittest failure on x86
 windows?
That test depends on the the precision of your system clock, though I'm baffled as to how it could be failing with the TickDuration.ticksPerSec >= unitsPerSec check in there - especially on seconds. Something about your system is triggering something that I've never seen before, though I suspect that it's more likely to be an issue with the test than the code being tested. Anything involving TickDuration is very hard to test properly, because TickDuration's precision depends on the system clock's precision.
 Does the autotester only run the tests with release binaries?
 Because it misses the assertion when compiled with x64 windows
 debug.
It runs them in both debug and release on the POSIX systems, but it may only run them in release on Windows. I vaguely remember something about that, but I'm not sure. - Jonathan M Davis
Dec 27 2012
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Jonathan M Davis:

 This is mostly up-to-date now: http://dlang.org/dstyle.html
I didn't know it was updated. I like it! :-) My code already follows all those rules (the only one I don't follow is "Braces should be on their own line", but that page it's only required for Phobos). Bye, bearophile
Dec 27 2012
parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, December 27, 2012 12:15:19 bearophile wrote:
 Jonathan M Davis:
 This is mostly up-to-date now: http://dlang.org/dstyle.html
I didn't know it was updated. I like it! :-) My code already follows all those rules (the only one I don't follow is "Braces should be on their own line", but that page it's only required for Phobos).
There are probably a few more tweaks that need to be done to it (e.g. I'd love to axe the section on comments, since we don't really follow it, and I think that it's unnecessary), but it's mostly correct now. - Jonathan M Davis
Dec 27 2012
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2012-12-27 12:05, Jonathan M Davis wrote:

 If you're fixing a bug, you need to have something along the lines of "fix
 issue# 777" in it. Beyond that, I don't think that there are any actual
 guidelines. Just be intelligent about them.
Then there's the standard Git guidelines. A commit message should consist of a short description, 50 characters max, followed by, if necessary, a blank line then a full description of the commit. If it's enough to describe the commit in 50 characters that's perfectly fine. -- /Jacob Carlborg
Dec 27 2012
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2012-12-27 16:15, Jacob Carlborg wrote:

 Then there's the standard Git guidelines. A commit message should
 consist of a short description, 50 characters max, followed by, if
 necessary, a blank line then a full description of the commit.

 If it's enough to describe the commit in 50 characters that's perfectly
 fine.
Not everyone is following this but it's good to try to follow it. The reason for this is that various git related software uses the first line to display it in a user interface or similar. For example, all git commits are sent to an email list as well. The first line is used as the subject of the email. Hence it's good if it's short. -- /Jacob Carlborg
Dec 27 2012
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, December 27, 2012 16:15:25 Jacob Carlborg wrote:
 On 2012-12-27 12:05, Jonathan M Davis wrote:
 If you're fixing a bug, you need to have something along the lines of "fix
 issue# 777" in it. Beyond that, I don't think that there are any actual
 guidelines. Just be intelligent about them.
Then there's the standard Git guidelines. A commit message should consist of a short description, 50 characters max, followed by, if necessary, a blank line then a full description of the commit. If it's enough to describe the commit in 50 characters that's perfectly fine.
It's my understanding that limit is 80, not 50. - Jonathan M Davis
Dec 27 2012