LDraw.org Discussion Forums

Full Version: What requires a HOLD vote on the Parts Tracker
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2 3 4 5 6 7 8 9 10
We've created this new forum that is only viewable by PT Authors and Reviewers to allow candid discussions regarding Parts Tracker issues. To kick things off, there has been some internal discussion about what, exactly should require a HOLD vote for a part.

Since I'm the one posting this, I'll start with my suggestions:

Things the should require a HOLD:
- Wrong part number
- Wrong or questionable origin
- Invalid syntax
- Blatantly wrong geometry

Things that should generate a warning but not a HOLD:
- Small gaps
- Wrong KEYWORDS or CATEGORY
- Semantics issues like, this should be a subpart
- Anything that the PT Admin can fix (e.g. Header issues)
- Minor color problems
- Basically anything that can be fixed after initial release without breaking models that use the part.

PT Admin shall have the final say on all Parts issues.
Officially we have only the outdated L3P error list:

http://www.ldraw.org/library/tracker/ref/l3pmsg/

while most of us use the output of Datheader (the following list was kindly provided by Mike Heidemann, author of the prog):


1    ERROR    Embedded POV Code found.    HOLD
2    ERROR    '0 WRITE' found. - HOLD
3    ERROR    '0 BFC CERTIFY INVERTNEXT' found.    HOLD
4    ERROR    '0 ROTATION' found.    HOLD
5    ERROR    '0 COLOR' found.    HOLD
6    ERROR    Incorrect use of only '0' for comments. Should be '0 //' instead.
7    ERROR    Not all used colors in LDConfig.ldr.    HOLD
8    ERROR    Matrix all zero found.    HOLD
9    ERROR    Identical vertices found.    HOLD
10    ERROR    Colinear vertices found.    HOLD
11    ERROR    Bad Vertex Sequence found.    HOLD
12    ERROR    Concave quads found.    HOLD
13    ERROR    Coplanarity.    HOLD
14    ERROR    Incorrect color for linetype.    HOLD
15    ERROR    Loop in reference found.    HOLD
16    ERROR    Double lines found.    HOLD
17    WARNING    Maybe wrong color for sticker used.
18    ERROR    Length of part description.    HOLD
19    WARNING    Maybe leading spaces in part description.
20    ERROR    Keywords entry length.
21    ERROR    Some keywords are also in part description.    HOLD
22    ERROR    Filename does not matches filetitle.    HOLD
23    ERROR    Author real name is not set.
24    ERROR    Author user name is not set.
25    ERROR    BFC is not set.    HOLD
26    ERROR    License is not set.    HOLD
27    ERROR    Part type is not set.    HOLD
28    ERROR    Primitives needs to have CCW winding.    HOLD
29    ERROR    Use of !KEYWORDS is not allowed for this part type.    HOLD
30    ERROR    Part description contains Tab character.    HOLD
31    ERROR    HELP entry length.
32    ERROR    Word 'new' or 'old' is used in part description.
33    ERROR    Use of (Needs work).    HOLD
34    ERROR    Entry for !CATEGORY.
35    ERROR    Use of !CATEGORY is not allowed at for this part type.
36    ERROR    High-res primitive has to start with '48\'.    HOLD
37    ERROR    Description for primitives should not start with '_' or '~'.    HOLD
38    ERROR    Filename for parts, shortcuts and primitives should not start with '48\' or 's\'.    HOLD
39    ERROR    Filename for subparts has to start with 's\'.    HOLD
40    ERROR    Description for subparts has to start with '~'.    HOLD
41    ERROR    Description for subparts should not start with '_' or '='.
42    ERROR    Description for parts should not start with '_' or '='.
43    ERROR    Description for shortcuts and/or physical_colour parts has to start with '_'.    HOLD
44    ERROR    Filename for shortcuts contains usually a 'c' or 'd'.
45    ERROR    Extension has to be .dat    HOLD
46    ERROR    Special characters in description not allowed.    HOLD
47    ERROR    Lines do not end with <CR><LF>.
48    ERROR    Author !HISTORY entry has no brackets.
49    ERROR    Not scalable primitive is scaled.
50    ERROR    primitive is scaled not only in Y direction.
51    ERROR    First line after BFC INVERTNEXT isn't linetype 1.
52    ERROR    Problem with the RGB value.
53    ERROR    '~Moved to' file used.
54    ERROR    Wrong BFC command found.
55    ERROR    Some keywords are used twice in KEYWORDS section.    HOLD
56    ERROR    'Minifig Accessory' found in part description.
57    ERROR    'Figure Accessory' found in part description.
58    WARNING    'INVERTNEXT' used although not necessary.
59    WARNING    TJunktion detected.
60    ERROR    Wrong brackets around username used. Only [] is allowed
61    ERROR    Description for alias parts should start with '='
62    ERROR    Description for Physical_Colour parts should mention the used colors
63    ERROR    Only linetype 0 and 1 are allowed in Physical_Colour parts.
64    ERROR    Wrong color (16 or/and 24) used for Physical_Colour part
65    ERROR    Only linetype 0 and 1 are allowed in Alias parts.
66    ERROR    Only one (1) linetype 1 is allowed in Alias parts.
67    ERROR    Only color 16 is allowed in Alias parts.
68    ERROR    Alias needs to be mentioned in the comments in the form 'Alias of partnumber'.
69    ERROR    Mentioned part number in the alias comment is wrong.
70    WARNING    Origin is outside the Boundingbox.
71    ERROR    American English words used in part description.
72    WARNING    Too much empty lines at the end of the file.
73    WARNING    Mirrored studs detected.
74    WARNING    Flat subfile scaled in flat direction
75    ERROR    Optional line with same vertices than line found.
76    ERROR    Leading or trailing zeros found.
77    ERROR    Overlapping triangle found.
78    ERROR    Lines with wrong number of arguments found.
79    WARNING    Filename ending does not match declared LDRAW_ORG file type.
80    ERROR    Moved To setnumber with Extension    HOLD
81    WARNING    Filename ending does not match declared LDRAW_ORG part type qualifier.


w.
(2017-12-08, 20:51)Willy Tschager Wrote: [ -> ]while most of us use the output of Datheader (the following list was kindly provided by Mike Heidemann, author of the prog):

While I like DATHeader, it's Windows only. I'd like to see a browser based solution. Ultimately, I'd like the PT to check this errors automatically. Both of these solutions are a long way off unless we get a volunteer coder to help us out.
I mostly agree that we are too strict overall... But another thing to factor in is the scarcity of reviewers. If I don't like something in a part I put a novote with comments, but there are so few reviewers, who could overwhelm my opinion, that a hold -  somehow forcing the author to react - would perhaps be better...
(2017-12-08, 20:58)Orion Pobursky Wrote: [ -> ]
(2017-12-08, 20:51)Willy Tschager Wrote: [ -> ]while most of us use the output of Datheader (the following list was kindly provided by Mike Heidemann, author of the prog):

While I like DATHeader, it's Windows only. I'd like to see a browser based solution. Ultimately, I'd like the PT to check this errors automatically. Both of these solutions are a long way off unless we get a volunteer coder to help us out.

I could possibly help with this.
I generally ask myself: is it necessary to fix the problem to release the part? If a wrong color is used, it will affect the end user a lot more than a missing contour. Concave quadrilaterals can cause misinterpretations by software so those, of course, must be fixed. A BFC error can cause potentially major rendering issues so those should always net a hold vote. A t-junction, that might cause some minor rendering artifacts and I try to avoid them in my work but I don't see why it should necessitate a hold vote. Or minor gaps or bleed-ins that generally are not visible unless you take a very close look at it.

I think that we often find problems (such as t-junctions) that would be nice to fix but shouldn't cause problems in by far most use cases. Currently these get a hold vote or, like Philo said, we keep back our certify votes for. The only current alternative is, right now, to put a "(Needs work)" qualifier which to me carries the connotation that the part requires major rework and we generally don't use it for simple problems.

We should keep in mind that generally people want these parts to build models with, and models have a scale much larger than what we as parts authors deal with. Our scale is 0.1 - 100 LDU or so in most cases; for models, 100 LDU is just the length of a 5 stud brick!

So perhaps we need better a platform for that? Maybe a list of issues (like a bug tracker of sorts) somewhere on the PT which would list parts that are released with issues, minor or major. Major issues are those that we currently apply the "(Needs work)" treatment to. Minor issues would then be ones that we can fix in a future update. Then we would have less issues to raise a hold vote for, or to keep back a certify for. Instead, release and put a note down that we should fix this later. This will probably create a backlog of things to deal with but that would still be better than a hundred parts held back for minor issues that aren't getting updates for years to come. Prioritize.

Perhaps there could also a third category for things that would be nice but what aren't realistic to deal with right away?

(nth) Edit: I also thought that a major part of the problem seems to be that when the part is released, it disappears from the parts tracker and needs to be re-submitted to it to be edited again, losing comment history in the process. So our only options are to release or keep it on the PT. I think that these shouldn't be mutually exclusive. A part with non-showstopper issues could be both released and kept on the PT for rework. A part with major issues can be released with a Needs Work modifier. Why does releasing a part remove it from the PT entirely anyway instead of just archiving it?
I don't think we should lower our standards only to get more parts released.
Making a list of parts with known issues is just a waste of time and effort. No one will ever rework that list of parts. Look at the list of parts with known BFC errors on the PT. No one was working it, untill I decided to do that.

What we need are more active rewiers and more responsive part authors. Uploading a part and not responding on a review is a waste of the reviewers time and effort. 
If I give a review in a Novote and the author ignores my opinion, I'm not likely going to review his next part. A second reviewer will ignore the file. Someone else has already revied it, and found some errors in it. Giving a Hold-vote is the only way to move things forward. I'm only allowed to change a bad file that have a Hold vote.

I also don't like the trend we are currently seeing. A sprint of hasty reviews only to get parts into the next release. Making parts take time, and reviewing them also takes time, and isn't something that should be done in the last minute. I prefer a more constant stream of releases, giving us a chance to quickly correct any faulty parts that needs recycling.
(2017-12-09, 14:18)Santeri Piippo Wrote: [ -> ]
(2017-12-08, 20:58)Orion Pobursky Wrote: [ -> ]While I like DATHeader, it's Windows only. I'd like to see a browser based solution. Ultimately, I'd like the PT to check this errors automatically. Both of these solutions are a long way off unless we get a volunteer coder to help us out.

I could possibly help with this.

For sure? I'd welcome this.

w.
I'm not concerned about the errors that can be fixed with a set of rules but when HOLDs are given based on "believe":

* I believe .... this is a subpart, part, shortcut, color xxx, ...

I think the final decision on part numbering, description and file type should be entirely up to the PT admin. Part authors might give suggestions but not HOLD them - 'cos with the system in place a HOLD cannot be overruled, not even with 10 CERTIFIED and an admin CERTIFIED. A HOLD has all the power a CERTIFIED has none.

w.
To proceed I suggest a hand down approach with revising Mike Heide's list as a starting point for Santeri Piippo if he is willing to code this up.

w.
Pages: 1 2 3 4 5 6 7 8 9 10