Month: October 2014

[WTH] critical flaw in our review process

Image Credit

code_review

Let me tell you a story

Recently, I have been assigned as a reviewer for a patch provided by our partner and this is okay to me because as a peer of Settings app, it’s part of my duty to review these patches. But … sadly, there are so many problems in that patch and I thought “Ok, it’s fine, because we all would make mistakes at first especially if you are not familiar with the part of codes”, so I tried my best to comment down all problematic codes with references (like to MDN or some related codes) to make sure they  can get familiar with them more quickly and easily.

After several rounds of this process, we finally made it and I thought this patch was good enough to be landed, so I gave them r+ on this patch.

If the story ended here, then I don’t have to write any bits of words like this article. As you may know, this is not the end.

What happened next

After signing-off this patch, there are still some other reviewing processes undergoing for different part of codes. For me, I have done my works, so I just left them alone and kept focusing on new features. Someday in the morning, I randomly checked the messages on Github and noticed there was one comment about the r+ patch, just out of curiosity, I decided to click it and checked it out.

That was a comment about missing change on the entry point and this definitely broke Settings app (You can’t even do anything). But … I was sure that this did work when I gave r+ because I did try the entry point to jump to that specific app when reviewing. So what’s going on right now !?

After a while, I finally realized that they just changed the code without any further notification and because the code change is kinda huge, no one would realize this problem when reviewing patch. So what does this mean ? If you really want to set someone up in this review process, you just have to get his/her r+ on your patch and use some magical git commands to break it and NO ONE WOULD NOTICE THIS AND WOULD BLAME ON THE REVIEWER !! (I wasn’t blamed yet, but if this patch got merged and I didn’t notice this, I would be the one)

Final

We all know this is the critical flaw in our review process, but in order to trust anyone, we haven’t forced people to tell us if the patch got changed after r+. By doing so, this would make the review process more easy and we don’t have to get stuck in some kinds of principles. Sadly, this happened to me few days ago.

Not sure what to fix here because if you are familiar with git, you can do whatever you want on commits. It is a double-edged sword which provides you so many advantages and also some shortages. I just hope this was not made on purpose …

SUCK.

[Nonsense] What’s going on recently ?

Image Credit

hi_there

It’s been a while not writing any article on my blog recently. For me, there are too many things happened within these two months no matter on life, works … etc. There are some good parts and also bad parts but I don’t want to talk too much on the later ones.

So, let me talk more about what good parts happened here recently.

Life

life sucks.

Works

Same as usual, there are too many things waiting to do / review in Mozilla. After last Friday, we just passed the due of 2.1 (but this doesn’t mean we have no 2.1+ blockers any more xD) and it’s like a “gap” now that we can refactor our old spaghetti  codes to make them clean and better.

For example :

  1. Removed some legacy code that would do the same thing in different scripts.
  2. Introduced an appsManager module in Settings app to make sure we won’t overload Settings app to keep access installed apps from API.
  3. Wrote lots of unit tests.
  4. Implementing Settings Dialog feature that we can easily use a function call to show/hide customized dialog with fancy animations.
  5. … etc

In addition to codes, I had to review partner’s codes back & forth and this reallllllly takes time … There is a huge communication gap between each others about due and how to implement features. No matter how, I think this part can get improved in later days.

And there is one more good news – We (All Mozilla employees) are going to Portland from 1st Dec ~ 6th Dec this year ! Can’t wait to go there to meet some new friends and also pass by LA to find my best friend !

Side Works

For side works,  I have focused mainly on Node.js (express 3 framework) and iOS development. I want to make an online service focusing on solving problems for backpackers when traveling and my friends and I are actively collect data for these stuffs (like hostels, food, drinks … etc).

In order to achieve this, I have to learn how to write with Swift for iOS app. I did write an app before (with obj-c) but it’s totally different from Swift in syntax and some concepts behind. But as a front-end developer, Swift looks more nature and easy to me but the only bad part for Swift is that you can’t find too many answers to specific problems because It is too young xD. But whatever, I like it so bad !

And for Node.js part, as a full-stack (I thought I was front-end) developer, you have to focus both on backend / front-end (including database, IT … blah). I am not really good at db / IT stuffs but I think this is such a good time for me to learn.(I did try some setup before but not so much) So, to conquer these, I did swipe my card on Linode to rent a machine and register one domain name for that service. By doing so, you can think this behavior of a resolution and you can’t go back ! (Because this would cost you money monthly XD)

I like the feeling to be pushed to the edge of cliff and this would force me to learn something new 🙂

Other plans

  1. I am planing to Thailand (Bangkok maybe ?) next year with friends.
  2. Have to keep reading books no matter what type it is ! (Just created a new page called my-bookshelf to track them :))
  3. Try to make Hax4 back so that this team can really work as a normal group !
  4. Keep learning something new.