Feedback on NI Community

cancel
Showing results for 
Search instead for 
Did you mean: 

Not Quite a Fan of the NI Verified Review

So I went looking for some of my old submitted code and I found some and saw that in the migration the formatting of the text was a bit messed up and so I went to edit when I saw that NI had added some text:

 

**The code for this example has been edited to meet the new Community Example Style Guidelines. The edited copy is marked with the text 'NIVerified'. Read here for more information about the new Example Guidelines and Community Platform.**

 

The link talks about the importance of style and how the code had gone through a VI Analyzer test and been updated.  Out of curiosity I opened the code and was mostly fine with the changes NI made.  But there are a few things that bugged me and felt like a discussion might be nice to have.

 

  • Block diagram size was not maximized, and was spanning two monitors.
    • Not all block diagrams should be maximized, and I usually don't.  But if you choose to do this for larger diagrams, then you at least ensure it doesn't span multiple monitors, or hang off of a monitor if you only have one.  NI took my submission, seemed to stretch the example to be wider than the 1440 of my monitor and saved it.  Not sure if this is intentional or not.
  • My original submission had two examples in it, but the NI one only has one.
    • I made two examples one that was basic, and one that was more advanced showing some more features of the API.  But this example wasn't included.
  • My submission relied on OpenG so I included a VIPC file with the OpenG packages required, but NI didn't include this.  
    • I think users like being able to double click a single file and have all the dependencies installed.  I'm not saying this should be a requirement but I put it in there for a reason and NI choose to not include it.
  • I'm not a fan of the statement that all VIs need to have debugging enabled.
    • I understand top level examples, but I have lower level APIs which might see performance improvements from having this.  This also some times allows for inlining.  If I turn off debugging in a VI it is for a reason.
  • Tiny low resolutions image
    • I don't remember what image I selected for my submission when I submitted it, but the one that was there after NI's edit was a very tiny and blurry resolution image for seemingly no reason.  There was one for the front panel of the main example and one for the block diagram.  I'd recommend something like the code capture tool which captures the image and the code with the full block diagram and front panel.  
    • I've edited my document to include the image from the code capture tool.
    • Tiny low resolution example images seems to be a problem with the tools network, as well.

I know that if I find some example code in the future, I'm going to choose to not download the NI Verified one.  I will not know if there was something NI choose to not include that the original author wanted to be there, like it was for me.  I don't think NI's intent was to spent the time reviewing code, only to have people prefer the non reviewed ones.  And this might be a rare case, and maybe the overwhelming majority of submitted code is crap and needs NI's review.

 

Over on LAVA we have a place for people to post code, and that code can be verified by someone at LAVA who checks it out for quality and style.  We don't change their submission, we just let them know for one reason or the other that their code failed the review and give them a chance to update the code and resubmit it, or just leave it as is for the public to have, knowing it doesn't meet LAVAs guidelines.  These stay in the Uncertified section.

Message 1 of 11
(8,660 Views)

Do you have a link to the code?

 

In any case, if code has been reviewed and changes made by NI, it seems absolutely required that the original author has a chance to review the review to make sure no new problems have been introduced.

 

Personally, I would trust original Hooovahh code more than something further processed by some random  NI intern who just finished LabVIEW 101. Reviewing code IS an important exercise to train interns, but there has to be some back and forth.

 

I would implement it more like peer review in science. A user submits an example and gets reviewer comments from a couple of independent reviewers. It is up to the author to implement the suggestions or argue why he is right. The code is accepted once all reviewers are happy with the corrected version.

 

(Maybe we could form an alternative agency that gives out badges for "Knight verified" or "Champion verified" code.:D)

 

 

0 Kudos
Message 2 of 11
(8,657 Views)
0 Kudos
Message 3 of 11
(8,638 Views)

Hooovahh,

 

Thank you for the feedback.  Let me first try to shed some light on the process, and then address this specific example.

 

Prior to the migration of examples from Jive to Lithium, there were no set rules or standards around the quality of code that was submitted, as well as the information about how to use it.  Looking at some of these submissions, they ran the gambit of being really poor designs and teaching users horrible coding practices to things that are super valuable. We wanted to improve this situation and created the Example Style Guide and associated VI Analyzer test.  We also recruited a number of coding enthusiasts from across NI, including some senior members of R&D, to act as gatekeepers and review any new code that gets submitted.  This review process is very similar to what you outlined on LAVA where it is collaborative and a back and forth between the submitter and the gatekeeper. If the submitter chooses not to make the recommended changes then the example is still available in the Draft area

 

However, as we put this new process in place, we needed to figure out what to do with the ~5,000 examples that were created before this process existed, some of them close to a decade old.  While we could have attempted to do a similar collaborative approach for these examples, the amount of overhead would have been extremely high, especially when it comes to code submitted by users who are no longer an active member of the community. For this situation, we decided to recruit a short term example scrubber team (not the same as the longer term gatekeeper role described above) to go through these older examples and update them to meet the style guide (leaving the original code in place), or in some cases recommend their removal if they are truly atrocious or duplicative. I also want to point out that this is not a team of NI interns as altenbach suggested.  Most of the members are at least CLD certified, and in fact, the specific scrubber who reviewed your code happens to have a CLED.

 

All that being said, as best we try, we know things will sometimes fall through the cracks, which seemed to have happened in this case where a number of key components were overlooked when updating the code.  I will make sure we communicate this to the individual scrubber, and work with him to ensure he is more diligent as he continues his work.  

 

Thanks!

 

Regards,

Hassan Atassi
Senior Group Manager, Digital Support
0 Kudos
Message 4 of 11
(8,590 Views)

@Hassan_Atassi wrote:

I also want to point out that this is not a team of NI interns as altenbach suggested! 


No, of course not. I was just exaggerating to make a point. 😄

(Maybe I should start using the Google Perspective tool to pre-wash my posts 🐵

 

In any case, if somebody would touch any of my old examples (have not checked), I want to know about it. If the original author is still active in the forum (or the e-mail is known), he needs to be notified.

 

And yes, I really appreciate all your efforts! The code example collection on Jive was absolutely abysmal and the few gems were completely drowned out in the noise of bad examples.

0 Kudos
Message 5 of 11
(8,573 Views)

@altenbach wrote:


No, of course not. I was just exaggerating to make a point. 😄

(Maybe I should start using the Google Perspective tool to pre-wash my posts 🐵 


That sentence seemed 3% toxic.  Really neat tool.  I wasn't notified and only noticed because I went to update my code and format.  I have since gone to a couple other things I have submitted and they haven't been reviewed yet.

Message 6 of 11
(8,563 Views)

I have been puzzled when I first encountered a NI-verified code in a sample program written in CVI. The document was this one, and going specifically I have found that the verified code is identical to the non-verified one, including the presence of CVIBUILD folder and .cdb and .exe files that are rebuilt from scratch every time the project is run.

I wonder what the verification process is for CVI code since, as far as I know, there is no style guide for developers. Even the very basic rule of adding comments to help users to understand the code is disattended in both versions of the example!

 

Summarizing: revised code is identical to the original one, no comments have been added, useless files are maintained, so what has the revision produced other that an unnecessary waste in space allocation?



Proud to use LW/CVI from 3.1 on.

My contributions to the Developer Community
________________________________________
If I have helped you, why not giving me a kudos?
0 Kudos
Message 7 of 11
(8,555 Views)

Let's have a look at the example program for charts (discussed here). This code belongs into the drafts, not in the approved area:

 

  • It claims that the chart starting time is somewhere in Dec 1903, without discussing the LabVIEW epoch or time zone setting. This seems relevant here because it is only true for a timezone with a certain specific negative offset from GMT). LabVIEW is used worldwide!
  • The first picture contains all irrelevant windows decoration, distracting from the code.
  • Code is ugly to say it mildly, for example why is the "To U32" inside the while loop? Yes, the compiler will move it out as loop-invariant code, but it is clearly something that needs to be done exactly once, and not with each iteration. Others are now copying that rube goldberg construct vebatim, thinking it must be correct.
  • Why are we doing 1/x and divide by x in parallel? We could just multiply the 1/x by 1000 instead. Less convoluted.
  • Setting the x-scale format in code is pointless. Once set, it will stay forever after the VI is saved. 
  • The diagram of the downloaded code contains a lot of whitespace and the window is offset, mostly outside the monitor area. Seems like it was originally maximized on a second monitor. That should be avoided.
  • What good is the error-out indicator if the loop starts no matter what?
  • ...

Should go back to "drafts"! 😮

 

0 Kudos
Message 8 of 11
(8,509 Views)

Thanks for the feedback altenbach.  Doing some investigation, it looks like this ended up in the wrong place during the migration.  We have moved it back to drafts 🙂

 

Regards,

Hassan Atassi
Senior Group Manager, Digital Support
0 Kudos
Message 9 of 11
(8,480 Views)

Okay coming back here with another complaint.  Previously I complained that one of my submissions went too far in the changes, now I'm complaining that I see other submissions haven't gone far enough.  I was looking through the idea exchange and through an idea found a Clock XControl.  I made a clock using picture controls years ago so I was curious to see how they did it.  I downloaded the NI Verified and started digging around.  I found a VI that was calculating some stuff so I opened context help and...

 

Untitled.png

 

Oh...that's odd let me just open the VI.

Untitled.png

That's right, no control or terminal labels.  I get that NI Verified doesn't mean it is great awesome code, just that NI did some basic checks on it.  But could "Has Control Labels" be one of the VIA tests they perform on it?  Thanks.

 

Edit: Oh and I just noticed the way they are drawing the clock means it will continue to use more memory forever.

0 Kudos
Message 10 of 11
(6,952 Views)