library

library copied to clipboard

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

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.

do we check if the code builds?

@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.

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

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

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

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.

...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...https://github.com/VCVRack/community/issues/389

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

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?

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.

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)

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

$ 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")

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

$ 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")

is this a capitalization thing?

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.

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")

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

Pinging people for #393

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? ...

@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.

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

Yup

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

Directly to master would be easiest for you.

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

still not sure how to solve https://github.com/VCVRack/community/issues/398#issuecomment-378751846 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.

@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

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