Hi Mika,

> > > That must be a new policy then, considering that stemodem 
> > is the only one that failed compilation. Feel free to fix 
> > this one. The first two patches in the set are unrelated to 
> > fast dormancy anyway.
> > 
> > that is the whole point here. You modified the enum and the 
> > compilation
> > should fail unless you add a statement for that new item.
> > 
> > Let me make this clear, I do want the compilation to fail here and the
> > STE driver is doing the right thing.
> 
> I understand that. As I said, feel free to fix. Fast dormancy patches 3-6 
> should apply just fine without the first two courtesy patches.

I am waiting for Denis review since he was looking at them initially.
Once these are applied, we can take care of the fallout.

> > There might be other cases where this is not consistent. I 
> > would prefer
> > if that never happens, but somethings things slip through even close
> > code review. If you know other cases, please let me know and 
> > we fix them
> > as well here.
> 
> All the other drivers implementing radio settings. Here's a couple of 
> examples:
> 
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 134)        switch (mode) 
> {
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 135)        case 
> OFONO_RADIO_ACCESS_MODE_ANY:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 136)                value 
> = 1;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 137)                break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 138)        case 
> OFONO_RADIO_ACCESS_MODE_GSM:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 139)                value 
> = 0;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 140)                break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 141)        case 
> OFONO_RADIO_ACCESS_MODE_UMTS:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 142)                value 
> = 2;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 143)                break;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 144)        default:
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 145)                
> CALLBACK_WITH_FAILURE(cb, data);
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 146)                
> g_free(cbd);
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 147)                
> return;
> ac63fd95 (Marcel Holtmann 2010-09-23 23:27:08 +0900 148)        }
> 
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 133)        switch (mode) 
> {
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 134)        case 
> OFONO_RADIO_ACCESS_MODE_ANY:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 135)                value 
> = 5;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 136)                break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 137)        case 
> OFONO_RADIO_ACCESS_MODE_GSM:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 138)                value 
> = 0;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 139)                break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 140)        case 
> OFONO_RADIO_ACCESS_MODE_UMTS:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 141)                value 
> = 1;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 142)                break;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 143)        default:
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 144)                
> CALLBACK_WITH_FAILURE(cb, data);
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 145)                
> g_free(cbd);
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 146)                
> return;
> 30b054d0 (Marcel Holtmann 2010-06-06 15:18:57 -0700 147)        }
> 
> Sorry, couldn't resist. ;-)

I have no problem with that being pointing out. I actually encourage
people pointing this out. This is clearly myself being stupid. And good
thing is that this is open source, so I fixed it.

With a code base of 100k lines of code and more, such things happen. And
even with Denis and myself cross-reviewing each other this will still
happen. Sometimes things just slip through the cracks. Part of
development life. So once noticed this will be dealt with.

Regards

Marcel


_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Reply via email to