Review team

[AndrewBelt]

The Review team is responsible for updating and adding plugin source code to repos/, making sure the source code is not malicious.

Workflow

Begin by checking this repo's issues for requests from plugin developers.

If someone requests an update to their build, first figure out what commit they're talking about in their source repo. Sometimes they'll mention a tag or branch or just say "the latest".

Next, cd into repos/<their plugin> and git pull, git checkout <tag>, or do whatever it takes to get the source they want into the submodule.

Then most importantly, review the source code for alarming stuff like system() calls, networking, modifying files, running stuff in the background, and anything else creative that someone can do. It's just as important to review their Makefile.
A "review" simply means gliding your eyes over the source code, or git diff in your terminal between the current commit and the last state of the submodule. TODO: What's a git command to compare the submodule's HEAD with the submodule's HEAD of the previous community commit?

If all is good, set "repoVersion" in the plugin's manifest JSON file to the VERSION string specified in the plugin's Makefile.
Don't change "latestVersion" or "status" in the manifest.

Finally, make a commit, and send a pull request (if you don't yet have VCV community access). In your commit message, say "reviewed" somewhere to acknowledge that the commit only contains reviewed source code.

Members

• @phdsg
• @mdemanett
• @cschol

[AndrewBelt]

Updated first post with Workflow section.

I previously mentioned the possibility of requiring proof of identification for being the Review team. That will not be required.

[phdsg]

do we check if the code builds?

[AndrewBelt]

@phdsg You can if you'd like, but it doesn't bother me if I attempt to build a repo and it fails to compile. (The build breaking on one arch but not another is a common case that will be unavoidable to fully detect before attempting to make builds.) I'll just make a comment in the plugin's thread that it doesn't compile on arch XXX because of a particular reason, they'll fix it, update the submodule again, and I'll rebuild. To save the extra steps, you can try building, but it's not required.

[AndrewBelt]

Thanks for the help so far guys! To organize team members, state your name and I'll list your username in the first post.

[phdsg]

@AndrewBelt does "branch" in .gitmodules have to point to the specific commit we checked out in the repo?

[AndrewBelt]

When you enter a submodule directory and check out a particular commit, that commit is saved when committing in the parent repository.

[phdsg]

yea, that worked for the previous PRs i did.
but with the moDllz i had a problem...
did what you just said. entered repo, pulled, checked out the commit...
after commiting and pushing, i still had unstaged changes in that repo i couldn't solve. some modified or deleted svg :(
so i reverted this one for now and try again.

[dllmusic]

...yesterday I found out there were unused svgs, (with same names but different cap)...so I cleaned them out to avoid issues... After that commit I posted the hash here...#389

[phdsg]

ah. this repo was already bumped to 0.6.1 before (but not yet made available)...
will take another look.

[phdsg]

anyways, for the future: how do we deal with situations like that @AndrewBelt ?
when a dev requests multiple updates between builds, one version bump per request or one per build?

[AndrewBelt]

Technically if a build hasn't been made yet (you can tell because latestVersion != repoVersion), the plugin developer doesn't need to bump the version unless the Build Team is in the process of making a build at the same time. So, accept the update but warn the developer, and if the new commit hash doesn't make it, the developer will complain after noticing that the build is out of date and we'll mention that they should have bumped the version.

[phdsg]

ok, tried once more to update the moDllz repo...
starting from clean working tree
cd repos/moDllz, git pull, git checkout 59c14cf, cd ../..
gets me here:

$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)

        modified:   repos/moDllz (new commits, modified content)

no changes added to commit (use "git add" and/or "git commit -a")

after git add repos/moDllz however, it still says:

$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        modified:   repos/moDllz

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)

        modified:   repos/moDllz (modified content)

[AndrewBelt]

When you git commit after adding the update, what does git status say?

[phdsg]

$ git status
On branch master
Your branch is ahead of 'origin/master' by 5 commits.
  (use "git push" to publish your local commits)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)

        modified:   repos/moDllz (modified content)

no changes added to commit (use "git add" and/or "git commit -a")

[AndrewBelt]

Looks like your changes were successfully committed. If you enter the moDllz directory, which content is modified? (git status)

[phdsg]

$ git status
HEAD detached at 59c14cf
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   res/moDLLzSwitch_0.svg
        deleted:    res/moDllzKnobM.svg
        deleted:    res/moDllzPort.svg

no changes added to commit (use "git add" and/or "git commit -a")

[phdsg]

is this a capitalization thing?

[AndrewBelt]

I think you're fine, right? Your local moDllz directory is out of sync which can be fixed with git clean -fdx and git reset --hard HEAD, but the community repo is now correctly linked with that commit number.

[phdsg]

after git clean -fdx and git reset --hard HEAD in the moDllz repo:

$ git status
HEAD detached at 59c14cf
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   res/moDLLzSwitch_0.svg

no changes added to commit (use "git add" and/or "git commit -a")

[phdsg]

yeah, seems this is a moDllz vs moDLLz filename conflict... hate that about windows!
pull request should be fine tho, right?

[AndrewBelt]

Pinging people for #393

[dllmusic]

Yes...there was some mixed files with same name and different caps.. (sorry guys about this).. I cleaned them up, but kept the filename.
Should I give the files a brand new name to make things easier? ...

[AndrewBelt]

@phdsg @mdemanett I've given you commit access so we can finally skip PRs.

Remember to acknowledge that you've "reviewed" the changes in the commit message if updating a submodule.

If someone would like to join the Review Team, send a few correct PRs following the Workflow in the first post, and I'll also give you commit access.

[phdsg]

does that mean we can work directly from a clone instead of the fork?

[AndrewBelt]

Yup

[phdsg]

nice, should we do our edits on branches or just directly to master?

[AndrewBelt]

Directly to master would be easiest for you.

I'll occassionally review commits but mostly just trust that things are in order.

[phdsg]

still not sure how to solve #398 (comment) and whether adding the repo with --depth=1 broke it or something else.

deleted my community fork and local clone of it, replaced it with a fresh clone from here.
all the modules check out ok.

[phdsg]

@AndrewBelt on the TODO thing in the OP:
not sure if there's a shortcut but i do:

• before updating the repo:
  git submodule status <submodule> to get the current commit hash
• and then in the submodule's repo after pulling the updates:
  git diff <commit> HEAD

[AndrewBelt]

Could someone review the list of open plugin threads and make sure we've done all we can do with the review process? This is probably a good link to bookmark. https://github.com/VCVRack/community/issues?q=is%3Aissue+is%3Aopen+label%3Aplugin+sort%3Aupdated-desc

[mdemanett]

I cleaned up a bit, closed a couple or commented.

AS and Impromptu (as of just now) are ready to build.

Bidoo remains outstanding on the build issue -- awaiting 0.6.1, I guess.

It'd seem helpful in general to clarify what people can do to be compliant with the build system when they're using libraries. That's been the biggest headache with the process so far. Like, if hypothetically someone had a good reason to depend on libsamplerate, and wanted to be in the build system, how would they do it?

[AndrewBelt]

As for libsamplerate, that library doesn't build on Linux using the cross compilers x86_64-w64-mingw32-g++ and x86_64-apple-darwin15-clang++-libc++ because of its sketchy build system (not because it's written sketchy but because autoconf for dumb reasons needs to run binaries that it compiles before it generates a Makefile, which is impossible since it's producing Mac/Windows binaries on Linux.) The best solution is to adopt libsamplerate into the plugin's own build system through SOURCES += libsamplerate/....

[AndrewBelt]

There's dead silence on the Facebook page about Plugin Manager sync issues.. which means this system is working well! I know how much effort it takes to review and curate these 116 plugins, so if you comment here with the best way to contact you privately, I'll grant your account all VCV-branded plugins, or $50 if you'd prefer for the past half-year in the VCV community.

[AndrewBelt]

@phdsg
@mdemanett
@cschol

[mdemanett]

Cool, been meaning to try those. I'm at matt at demanett dot net. Thanks!

[cschol]

Hey @AndrewBelt, sorry, I had missed the notification for message above from June 5th. My email is modular80 at gmail dot com. Thank you!

[c50a326]

Hey, this is the best place I could find to ask this:

How is the open-source/closed-source thing working right now? I mean, how are closed-source plugins added to the community plugins, and how can they be verified as not being malicious in the absence of the source code?

Let me know if there's a more appropriate place for this question/discussion, or somewhere it has been had before, thanks!

[AndrewBelt]

@c50a326 If you're interested in publishing a closed source plugin, see https://github.com/VCVRack/community/blob/master/README.md#addingupdating-your-plugins-build-for-closed-source-free-and-commercial-plugins

If you're interested in your own security as a user, VCV has government IDs on file before allowing submissions of unreviewed files.

[Coirt]

Just so everybody is aware of making changes after automation queuing a repo for review / build or up to date status. What are the proper procedures regarding this. There have been a few instances, recently and in the past, where the integrity of Rack have been compromised by updates. This is rare and not to point the finger at anyone, these things happen...

What would be the best course of action for a Plugin Dev to take if there is a last minute bug or change? Would simply closing the issue then submitting a new build be the best way make sure a compromising commit does not make it through?

[cschol]

Github issues are not connected to the build. We just use them to track the work to be done. As soon as I commit a version of a plugin to the repository, the next build run will pick it up. To prevent it from being picked up means that either myself or @AndrewBelt has to revert the commit in the library repository. Closing issues won't do anything, except cause potential confusion on our part.

In general, the problem is one of quality control on the plugin developer's side. Developers should be submitting their plugin for release AFTER quality control has been done on their side. A lot of developers of open source plugins do this by posting pre-release versions in the forum, which is great. You get some runtime on your plugin and people can test before releasing it via official channels. Every one of those critical issues could have been prevented with a little more testing before submission.

To answer your question: notifying us via Github issue, or better email, is probably the best course of action in a case like you described.

Any thoughts on this, @AndrewBelt?

[AndrewBelt]

Closing since the VCV Community forum is a more organized place for discussions.