Page MenuHomeFreeBSD

Update review requests for bhyve.
ClosedPublic

Authored by jhb on Nov 5 2018, 5:54 PM.

Details

Summary
  • Explicitly mention the bhyve group on Phabricator.
  • Request reviews of the userland components (libvmmapi, bhyve, bhyvectl, and bhyveload).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

araujo added a subscriber: araujo.

Sounds reasonable!

This revision is now accepted and ready to land.Nov 5 2018, 6:01 PM

Seems to achieve the desired policy goals.

This is good, but it is not clear on *who* can approve as bhyve, in the past that was Peter, is it now jhb & tychon, or anyone who just happens to join the bhyve group, which is not controller.

This is good, but it is not clear on *who* can approve as bhyve, in the past that was Peter, is it now jhb & tychon, or anyone who just happens to join the bhyve group, which is not controller.

If the bhyve group cannot be controlled, in this case would be better add the team members name explicitly.
I thought the bhyve group were controlled.

This is good, but it is not clear on *who* can approve as bhyve, in the past that was Peter, is it now jhb & tychon, or anyone who just happens to join the bhyve group, which is not controller.

If the bhyve group cannot be controlled, in this case would be better add the team members name explicitly.
I thought the bhyve group were controlled.

It is controlled to the extent that only a committer can join it, but any committer who has joined it well get the bhyve umbrella accept check by default if they are a member and do an accept of a bhyve review. This could be changed. It is just how allanjude set it up when it was created.

araujo requested changes to this revision.Nov 6 2018, 3:53 AM

After reflect a little bit more about what @rgrimes mentioned, how about we do something like this?

usr.sbin/bhyve* tychon, jhb, anish, araujo, rgrimes <...> Pre-commit review requested via #bhyve phabricator group.

It would applies also to libvmmapi and friends.

This revision now requires changes to proceed.Nov 6 2018, 3:53 AM

I think listing a lot of folks will start to get unreadable. I think we can start with the existing language and just use some common sense (if a couple of folks who aren't random committers approve userland changes, then that's probably fine). If we have to nail it down further in the future we can revisit it then.

I am good with the language as it is for now, lets get this commited sooner rather than later please.

I still feel locked out of usr.sbin/bhyve, mostly of the commits made there in the past year were made by myself or reviewed by myself. Would be fair have my name explicit in that area, I don't want bypass reviews, it is more about the effort I have put in that area and appreciation of it, otherwise doesn't make sense for me to keep contributing with bhyve.

I'm approving with disapproval.

This revision is now accepted and ready to land.Feb 8 2019, 10:06 PM

Would it make sense to add araujo's name to the usr.sbin/byhve line?

This revision was automatically updated to reflect the committed changes.