When to yank commit privileges?

[Elixer]

Here is the story, there was a issue with some code, and the issue was fixed, removed over 800 lines of useless code that we would never use, and then said patch was put on a tracker for further comments on.

Another person comes along and knowing the patch was on the tracker still proceeded to 'update' the old library.
His push did NOT fix the issue at all, nor did it do anything except update said library to newest version and some docs.

I feel that, since they very well knew about the patch on the tracker, and yet still proceeded to update said code with trivial updates, that they shouldn't be allowed to have commit privilege anymore.

This person isn't even a programmer at all, they had access for unrelated things that have nothing to do with the overall code base, just a few maintenance tasks.

Haven't gotten a explanation for the commit yet, but this is a pretty silly thing to do.

So, I am curious, how many people would yank their commit privs, and instead force them to submit patches to the tracker instead ?

 

[DaveSimmons]

Haven't gotten a explanation for the commit yet, but this is a pretty silly thing to do.

I'd need to know this, and also to know why they currently do have commit privileges. What are the maintenance tasks?

 

[Elixer]

I'd need to know this, and also to know why they currently do have commit privileges. What are the maintenance tasks?

No comments yet about the issue.
They are basically maintaining a different build system as a courtesy mainly.

 

[Ken g6]

What kind of version control system do you have? With some, like Git, it should be possible to sideline this user fairly easily. Then again, with Git, merging the tracker update and the user's update shouldn't be hard either.

 

[Elixer]

What kind of version control system do you have? With some, like Git, it should be possible to sideline this user fairly easily. Then again, with Git, merging the tracker update and the user's update shouldn't be hard either.

Using git, and that is exactly my thought.
There is no valid reason for them to have commit privs since they are abusing them, so they will have to submit patches, and, after review, we will apply those patches.

 

[Ken g6]

No, my thought is not your thought. My thought is that you should just take this user's commit and leave it on a side branch until you're ready to merge it. (Which can be never if you like. )

At least, don't do anything rash until you find out why they submitted the commit.

 

[Elixer]

You can't limit them to a certain branch with git, it is commit access for all or nothing.
I guess another alternative is to make another repo, just for them, and do merges from that, but, patches are easier, and this is what the kernel guys do, and it seems to work out well for them...

 

[veri745]

You can't limit them to a certain branch with git, it is commit access for all or nothing.
I guess another alternative is to make another repo, just for them, and do merges from that, but, patches are easier, and this is what the kernel guys do, and it seems to work out well for them...

You don't have to make "another repo"

By limiting access of who can push changes, and where changes are being pulled, you can control what you need to.

http://git-scm.com/book/en/Distributed-Git-Distributed-Workflows

 

[Elixer]

Right, I know how git works, and in this case, all programmers, and this person has push access to the same repo.
What you suggest can't fix the current situation, we don't have a 'dictator' or a integration manager.
That means, he needs to either clone the repo, then we can pull their stuff when needed, or just do patches.

 

[Elixer]

Their response was "I needed to update it."
That was it.
...
sigh.

 

[beginner99]

Whats the discussion. he isn't a programmer and obviously pretty much clueless. I would not let him anywhere near the code.

 

[Cogman]

Whats the discussion. he isn't a programmer and obviously pretty much clueless. I would not let him anywhere near the code.

My thoughts exactly. My first thought was

"Well, you revoke it when you fire someone, otherwise train, train, train"

But then when I came across the

"He isn't even a programmer"

My thought was

"Why the hell does he have commit privileges?"

If he needed to have the version of something updated, the right and proper way is for him to submit a case to the issue tracker and let whoever maintains that code worry about the update, especially since they will be much more intimately informed on what is going on with the code base.

Non-programmers shouldn't have access to change code, period, end of story. As shown here, they can do a ton of damage and don't even have the basic skill of fixing their own mess.

 

[smackababy]

The question is who needs access and who has it that doesn't need it? I think I'd talk to management about revoking access to all non developers.

And the amount of damage is a bit of an exaggeration. Anyone using even the most rudimentary source control and revert the changes he made with ease.

 

[Cogman]

The question is who needs access and who has it that doesn't need it? I think I'd talk to management about revoking access to all non developers.

And the amount of damage is a bit of an exaggeration. Anyone using even the most rudimentary source control and revert the changes he made with ease.

Eh, good point, my "tons of damage" was hyperbole. But it does suck to revert changes if someone screws something up and new things are built on top of the screw up.

 

[Schmide]

My 2 rules for source.

You have to be able to compile to check in and you must check out before you check in.

I call the second the bizaro hotel rule.

 

[smackababy]

My 2 rules for source.

You have to be able to compile to check in and you must check out before you check in.

I call the second the bizaro hotel rule.

Why would you check in anything if you can't compile and test locally first?

And, depending on what source control you use, a checkout might simply be the ability to get a file. Some programs like PVCS allow you to lock a file while you have it checked out and some just treat a checkout as a get.


We have a rule that if you cause a build failure on the continuous build server (from broken code check in), you have to put money in a jar. We aren't exactly sure what the money is going to be for though.

 

[Schmide]

Why would you check in anything if you can't compile and test locally first?

There are cases where non-programers check in stuff. For example I worked on a project were one guy, a graphic artist, that was the interface designer. He would design dialogs and such using the gui and someone else would fill in the code. Changing/removing IDs can break a build. We forced him to learn how to check out then compile before checking in.

This lead to another strange activity. We're taking a break. This was when we announced that the source was broken while 2 or more people checked in conditional code and arbitrated their changes.

As for compiling locally, the bizarro hotel rule, is basically saying you have to test on the latest version before you check in. Do your merges locally not globally.

 

[Apathetic]

Their response was "I needed to update it."
That was it.
...
sigh.

They need to understand that WHEN they commit changes matters. They can't just commit a change when they feel like it. Help them understand the damage they caused by doing this.

How often do they commit changes? If it's not very often, the simplest thing would be to have them email you for permission before they do the commit. That being said, if they do it again, revoke their privileges.

Dave

 

[Obsoleet]

I'm a hobbyist dev, until next year when I switch roles to a fulltime developer. And I've always run the show on our source control at work. I've never 'needed to update' much code though, so I never have.
I would say a good rule of thumb is to never allow any non-devs to have commit privileges.. especially if you're hiring guys like you described.

Don't trash the guy for doing what he did though. I wouldn't lay the blame at his feet at all. It's your fault or your management's fault that he even had this access. For example, I run the show in my office and have never had anything like this happen- because only devs currently working on the project have access.
But I'd fix the original f-up and revoke the privileges.

 

[Elixer]

Don't trash the guy for doing what he did though. I wouldn't lay the blame at his feet at all. It's your fault or your management's fault that he even had this access. For example, I run the show in my office and have never had anything like this happen- because only devs currently working on the project have access.
But I'd fix the original f-up and revoke the privileges.

They already had access before I was there, and apparently, I am not the only one to complain (seems this did happen before as well--looking at the logs, looks like they do it every 2-3 months for the past 3 years), but nobody bothers to follow up on it besides telling them to "not do it again".
Since I was picked to manage it, he has no access anymore, I just don't trust someone who keeps making the same mistakes every few months leaving it to other people to fix their mess.