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 

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

 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

 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